- Issue created by @KeyboardCowboy
- Status changed to Active
8 months ago 11:06pm 29 March 2024 - π·πΊRussia kostyashupenko Omsk
Would be nice to clarify this ticket. We don't have so granular components in Navigation module, like for instance component "Text" with 50 variations. I think this ticket is more about review of the current styles for the all texts?
- π·πΊRussia kostyashupenko Omsk
Also i see so many text variations on design. Are they all 100% used in Navigation module? Basically the scope only is sidebar + top bar. Did i miss something?
- πͺπΈSpain ckrina Barcelona
I've given a thought to it and yes, we should be using only the ones the navigation needs, but with our own abstraction level. I've updated the issue summary accordingly. Sorry @KeyboardCowboy for the back and forth!
These are the ones we're using:
- Info LG (heavy) (18px) - Drawer title (
--admin-toolbar-font-size-l
) - Info SM (heavy) (16px) -> Level 0 and 1(
--admin-toolbar-font-size-base
) - Info XS (Medium) (14px) -> Level 2 (
--admin-toolbar-font-size-s
) - Info XS (14px) -> Level 3< (
--admin-toolbar-font-size-xs
)/li> - Label (Uppercase) XS (10px) -> Menu/block label (
--admin-toolbar-font-size-xxs
)
We can keep the font sizes we have, but I would recommend defining a layer of abstraction with the following variables in case anybody in the future wants to easily customize the navigation. I know it might sound as an overkill right now, but if we want to let users customize the admin UI in the future we better start implementing easily overridable values now.
--admin-toolbar-font-size-block-title --admin-toolbar-line-height-block-title --admin-toolbar-font-size-level-0 --admin-toolbar-line-height-level-0 --admin-toolbar-font-size-level-1 --admin-toolbar-line-height-level-1 --admin-toolbar-font-size-level-2 --admin-toolbar-line-height-level-2 --admin-toolbar-font-size-level-3 --admin-toolbar-line-height-level-3 --admin-toolbar-font-size-popover-header --admin-toolbar-line-height-popover-header
- Info LG (heavy) (18px) - Drawer title (
- Status changed to Needs review
8 months ago 8:52am 2 April 2024 - π·πΊRussia kostyashupenko Omsk
1. Created list with font-size, line-height, font-weight and letter-spacing CSS variables.
2. Font-weight - because potentially someone will want to customize font. And we have 400, 500, 600, 700 font weights in our design system. Potentially custom font will never have some of those weights, so it will be easy to override font weights from css.
3. Letter spacings are also documented. Just to be able to customize it (coz it's pretty dangerous css property, which is working "fine" only with current font, but will not work with custom fonts)
4. Reworked tooltip a bit (now fully matching Figma)
5. Didn't touch breadcrumbs. We still have code related to breadcrumbs, but idk about the current state of this feature. I remember we did an override of breadcrumbs with some custom styles, but not sure if it's still actual feature or forgotten.I suggest you to quickly review. In case of minor feedbacks -> let's create follow-up issue, because potentially we can get some (or lots of) conflicts.
- Status changed to Needs work
8 months ago 10:02am 2 April 2024 - πͺπΈSpain ckrina Barcelona
I've just added feedback bout the extra layer of variables for weight, which is not needed on a design system perspective.
- Status changed to Needs review
8 months ago 7:18am 3 April 2024 - π·πΈSerbia finnsky
I don't think toolbar-button should be related to levels somehow.
- Status changed to Needs work
8 months ago 8:45am 3 April 2024 - π·πΊRussia kostyashupenko Omsk
Re #11
i think i made a mistake. Because seems on design 0-level links and buttons are equal to 1-level links and buttons in terms of text - Assigned to ckrina
- πͺπΈSpain ckrina Barcelona
@finnsky you're right, I forgot the rest of elements that will come into the Top Bar. I'll take a go on this and replace this with a normal design system variables. Sorry @kostyashupenko, fo changing it. I'll work on this to close the design implementation work.
- Issue was unassigned.
- Status changed to Needs review
8 months ago 11:06am 3 April 2024 - π·πΊRussia kostyashupenko Omsk
mmm @ckrina i have pushed one little commit to get rid of `toolbar-button--level-1` selector.
But i also left a comment in MR. We have so many CSS variables for fonts which will never be used seems like. Tagging to `Needs work` for discussions
- π·πΊRussia kostyashupenko Omsk
I have resolved my feedback on MR, but once i have pushed new commit, we need again someone to review everything
- Status changed to Fixed
8 months ago 12:37pm 4 April 2024 - πͺπΈSpain ckrina Barcelona
Thanks for the feedback and questions, I'll keep them in mind to prepare an explanation for it because others will have the same questions. Merging this meanwhile to prepare a release.
Automatically closed - issue fixed for 2 weeks with no activity.