search

generoi / genero-design-system

https://gds.generogrowth.com/
MIT License
4 stars 0 forks source link

A11y #41

Closed oxyc closed 3 years ago

oxyc commented 3 years ago

There are at least two bugs.

  1. ~Not sure where the styling for the submenu went?~

  2. ~This styling is not ideal but it's quite complicated to get it correct....~

There's one annoying breaking change here: https://github.com/generoi/genero-design-system/commit/faf842434449adb3728bc780577a72384dd31cfb

The above commit defaults menus to align left. We could also default it to align-center, then the <gds-navigation> component would look correct but the <gds-menu> used directly would be centered instead of aligned left. This is less of a breaking change but I think it's a weird default.

This should make all the components tabbable correctly and have basic accessibility but it still needs real life testing. The menu could also use keyboard navigation beyond tabbing but I don't think that's a strict requirement since it does work through tabbing.

oxyc commented 3 years ago

TLDR; I moved this to a separate PR https://github.com/generoi/genero-design-system/pull/51

I found it difficult to get the proper focus box sizes when there were quite many wrappers around everything. So in commit https://github.com/generoi/genero-design-system/pull/41/commits/4c7edc9540c891b7cf4fc4aa25abd35a2ff2e5a6 the focus styling on menu-item-nested looks good and most components got simplified.

This is quite opinionated though, I personally prefer having fewer wrappers around. What do others think? I can easily drop this commit and we can look at the focus style boxes later since it's not very important.

Here are the relevant diffs of the DOM change: https://github.com/generoi/genero-design-system/pull/41/commits/4c7edc9540c891b7cf4fc4aa25abd35a2ff2e5a6#diff-639cc5987fa2b5be60df8950700bb506e781a89d6f0b0d84170094216e7f8c28L98-R118

https://github.com/generoi/genero-design-system/pull/41/commits/4c7edc9540c891b7cf4fc4aa25abd35a2ff2e5a6#diff-569ecd2879518dceeefe2e932603fb2984fc28a0e2034c086a5fbf133a0a0360L36-R38

https://github.com/generoi/genero-design-system/pull/41/commits/4c7edc9540c891b7cf4fc4aa25abd35a2ff2e5a6#diff-7b5bcf3c223e6f4f72ca8f9220883641981b01e0beaef0a2374784c14156f175L37-L44

https://github.com/generoi/genero-design-system/pull/41/commits/4c7edc9540c891b7cf4fc4aa25abd35a2ff2e5a6#diff-260768a9e14dfcfd429f0bfbc3b9496ef4d62f63fa101c02e0cf437405665247L34-L38

This commit also has another breaking change:

BREAKING CHANGE: --menu-item-underline-display has been replaced by --navigation-underline-active and the underlines are no longer spanning the entire block, only the text content.

oxyc commented 3 years ago
Screen Shot 2021-03-28 at 14 49 33

Updated the styles to use outline, we'll live without support for border radius.

oxyc commented 3 years ago

4.0.0-beta.0 (2021-04-02)

⚠ BREAKING CHANGES

--menu-item-underline-display has been replaced by --gds-menu-item-underline-active and the underlines are no longer spanning the entire block, only the text content. Additionally you can modify the offset and thickness; and --navigation-underline-color is now called --gds-menu-item-underline-color-active.

// BEFORE:
--menu-item-underline-display: none;
--navigation-underline-color: var(--color-black);

// AFTER:
--gds-menu-item-underline-active: underline;
--gds-menu-item-underline-thickness-active: 3px;
--gds-menu-item-underline-color-active: currentColor;
--gds-menu-item-underline-offset-active: var(--spacing-xxs);

Menu variables renamed

All --menu-* variables have been prefixed with --gds-menu instead.

// BEFORE:
--menu-text-align: left;
--menu-item-background-color-active: var(--background-color-primary);

// AFTER:
--gds-menu-text-align: left;
--gds-menu-item-background-color-active: var(--background-color-primary);

Markup change for ´`

Using <gds-menu-nested-item> should now wrap the link content in <gds-menu-item>.

 <gds-menu>
   <gds-menu-item-nested slot="item" submenu-icon="❯">
     <a slot="link" href="#first">
-      First item
+      <gds-menu-item>First item</gds-menu-item>
     </a>
   </gds-menu-item-nested>
 </gds-menu>

With the above change, the --gds-menu-item-nested-a-padding variable has now been removed and the padding is instead styled from the <gds-menu-item> component.

// REMOVED:
--gds-menu-item-nested-a-padding: var(--spacing-xs) 0;

Navigation gutters

Durings this refactoring the fixed menu height that used to be set var(--spacing-xxl), is now fluid and instead uses the following variables for adjusting the padding on <gds-menu-item> depending on the direction of the <gds-menu> component:

--gds-menu-item-padding-horizontal: var(--spacing-s) var(--spacing-s) calc(var(--spacing-s) - 0.15em) var(--spacing-s);
--gds-menu-item-padding-vertical: var(--spacing-s) var(--spacing-m) calc(var(--spacing-s) - 0.15em) var(--spacing-m);

This is likely to break some implementations or at least change the height of the navigation by a few pixels.

The navigation containers now supports --navigation-padding to consistently set left/right padding. You most likely need to change some theme styles for the right padding of the navigation as well as reduce the left padding for the navigation logo --navigation-logo-padding. By default the --navigation-padding sets the same values as before, making spacing identical as it used to on both desktop and mobile. If you override the variable you should consider the spacing both for desktop and for mobile.

// Before
--navigation-logo-padding: 0 0 0 var(--spacing-s);

// After
--navigation-logo-padding: 0;

Navigation hamburger icon

The navigations default mobile hamburger has now been changed to an animated <gds-hamburger> component, if you applied color styling to this in your theme you might need to make adjustments.

Navigation menu item justification

<gds-menu> used within a <gds-navigation> needs to specify that the menu items should be centered to maintain the previous behavior otherwise the menu will be aligned to the left.

Add this to your css:

// NEW VARIABLE:
--gds-menu-justify-content: center;

<gds-input-wrapper> no longer using shadow-dom

Due to accessibility issues and it not being possible to reference input fields in a slot from a <label> inside a component, the <gds-input-wrapper> is now using light-dom and scoped styles instead of shadow-dom. This could cause high specificity global styles to pollute the component.

Features

Bug Fixes

taromorimoto commented 3 years ago

@oxyc Great release notes! ❤️

Is there a reason you are omitting the --gds prefixing variable-names branch?