Problem/Motivation

The typography in the navigation should be set to match the Figma designs. Update the font sizes, line heights and font weights.

  • 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)

Proposed resolution

Keep the font sizes and weights we already have, and correct line heights.

Also, define a layer of abstraction with the following variables in case anybody in the future wants to easily customize the navigation.

--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

Current definitions that affect us from the design system:

Source URL for Figma

Issue fork navigation-3435981

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

KeyboardCowboy created an issue. See original summary.

KeyboardCowboy’s picture

Title: [PP-1] Replace typography with Figma Styles » Replace typography with Figma Styles
Issue summary: View changes
Status: Postponed » Active
Issue tags: -Needs design
StatusFileSize
new95.97 KB
new53.46 KB
new40.92 KB
new26.17 KB
new46.23 KB
KeyboardCowboy’s picture

Issue summary: View changes
kostyashupenko’s picture

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?

kostyashupenko’s picture

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?

ckrina’s picture

Issue summary: View changes

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

kostyashupenko’s picture

Status: Active » Needs review

1. Created list with font-size, line-height, font-weight and letter-spacing CSS variables. <-- this one wasn't just replaced. Now actually we have fully aligned styles with Figma design (previously some of the texts had wrong font-size, or weight, or line-height props)
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.

ckrina’s picture

Status: Needs review » Needs work

I've just added feedback bout the extra layer of variables for weight, which is not needed on a design system perspective.

kostyashupenko’s picture

Status: Needs work » Needs review
finnsky’s picture

I don't think toolbar-button should be related to levels somehow.

Probably we just should add simple typography variables?

In some simple logic like:

https://m3.material.io/styles/typography/type-scale-tokens

That "levels" sounds unexpected.

kostyashupenko’s picture

Status: Needs review » Needs work

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 styles

ckrina’s picture

Assigned: Unassigned » ckrina

@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.

ckrina’s picture

Assigned: ckrina » Unassigned
Status: Needs work » Needs review

Ready for review!

kostyashupenko’s picture

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.

kostyashupenko’s picture

I have resolved my feedback on MR, but since i have pushed new commit, we need again someone to review everything

ckrina’s picture

Status: Needs review » Fixed

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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.