Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DropdownMenu v2: Design tweaks #55933

Closed
63 tasks done
Tracked by #50459
ciampo opened this issue Nov 7, 2023 · 8 comments · Fixed by #56041
Closed
63 tasks done
Tracked by #50459

DropdownMenu v2: Design tweaks #55933

ciampo opened this issue Nov 7, 2023 · 8 comments · Fixed by #56041
Assignees
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Comments

@ciampo
Copy link
Contributor

ciampo commented Nov 7, 2023

dropdown.mp4

Some design details may be subject to change, but the full spec for updated MenuItems/MenuItemFlyouts can be found in Figma here.

The prototype in the video above can be tried here.

List of tweaks

Click to expand
  • Remove group label
  • Expose Help Text subcomponent, including truncation
  • Allow nested triggers to have a suffix while preserving the chevron
  • Tweak submenus overlap
  • Add auto-indent functionality
    • Using context
    • Using CSS grid
  • When suffix is not present, children fill all available space
  • Add more examples to Storybook:
    • Indented / unintended
  • Review sizes:
    • label font size
    • help text font size
    • chevron size
    • check and radio size
    • icon size
  • Review spacing:
    • menu padding
    • item spacing
    • divider spacing
    • min and max menu widths
    • prefix spacing
    • suffix spacing
    • chevron spacing
  • Review all text/bg colors on
    • Label:
      • resting
      • hover
      • focus
      • disabled
      • active flyout
    • Help text
      • resting
      • hover
      • focus
      • disabled
      • active flyout
    • Prefix
      • resting
      • hover
      • focus
      • disabled
      • active flyout
    • Suffix
      • resting
      • hover
      • focus
      • disabled
      • active flyout
    • Chevron
      • resting
      • hover
      • focus
      • disabled
      • active flyout
    • Bg color
      • resting
      • hover
      • focus
      • disabled
      • active flyout
  • Review border and separator color, including drop shadow
  • Truncate label text

Questions:

  • are there reusable line-height variables?
  • Box shadows: spec vs existing popover box shadow?
  • border colors: spec vs existing variables?
  • toolbar variable?
@ciampo ciampo changed the title Design tweaks DropdownMenu v2: Design tweaks Nov 7, 2023
@jordesign jordesign added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Nov 8, 2023
@afercia
Copy link
Contributor

afercia commented Nov 16, 2023

Feedback from #56035
Cc @jameskoster

It appears the new spec doesn't provide a specific style for dropdown menu items that are selectable and thus have a pressed/active state and use both an icon and visible text.

There are cases in the editor where icon + visible text is already in use, for example the Align dropdown. #56035 uses icon + visible text for the Heading levels as well. Screenshot:

Screenshot 2023-11-16 at 14 32 54

Worth also noting that the Button component, which is used in the Dropdown, does have a 'pressed' styling for icon + text. However, it works with the is-pressed CSS class, but it doesn't with the is-active CSS class. See the storybook: https://wordpress.github.io/gutenberg/?path=/story/components-button--icon&args=isPressed:!false;text:Some+text;aria-pressed:true

Screenshot:

Screenshot 2023-11-16 at 14 21 46

@jameskoster
Copy link
Contributor

In the context of a menu the highlighted icon is a bit unconventional compared with checkmarks and radios, it's not always obvious what it means. This approach will also come under scrutiny when we get around to implementing dedicated light/dark modes in the future too.

Placing the selection state in it's own 'column' helps identify toggled items at a glance. Consequently text is indented and aligned across the whole menu.

Those points considered, if selectable items with icons were supported, the following could result, which is quite clumsy in appearance:

Screenshot 2023-11-16 at 14 44 56

For examples like alignment and heading level, isn't there a case to be made that the icons are adequately descriptive on their own (with appropriate labels and tooltips)? And in cases where the icon isn't descriptive enough on it's own, then it's probably not adding much value, and a text-only application is appropriate.

@ciampo
Copy link
Contributor Author

ciampo commented Nov 16, 2023

Worth also noting that the Button component, which is used in the Dropdown, does have a 'pressed' styling for icon + text. However, it works with the is-pressed CSS class, but it doesn't with the is-active CSS class. See the storybook: https://wordpress.github.io/gutenberg/?path=/story/components-button--icon&args=isPressed:!false;text:Some+text;aria-pressed:true

The new DropdownMenu component will add new subcomponents specifically targeted at implementing radio and checkbox menu items, which won't use the Button under the hood.

Nevertheless, what you're flagging could be seen as a general improvement that we can make to the Button component, so I'd still encourage you to log it as a separate issue

@afercia
Copy link
Contributor

afercia commented Nov 20, 2023

The new DropdownMenu component will add new subcomponents specifically targeted at implementing radio and checkbox menu items, which won't use the Button under the hood.

Will it still use a <button> element though?
@ciampo can you please point me to the relevant discussion and PR (if any)?

@afercia
Copy link
Contributor

afercia commented Nov 20, 2023

For examples like alignment and heading level, isn't there a case to be made that the icons are adequately descriptive on their own (with appropriate labels and tooltips)? And in cases where the icon isn't descriptive enough on it's own, then it's probably not adding much value, and a text-only application is appropriate.

Repeating here the accessibility feedback already provided on #56035
For accessibility reasons, visible text is always preferable. Text is the only 'universal' format to provide information that can be understood by everyone. The usage of icon-only controls should be limited to only the cases whre there is no sufficient horizontal spece (e.g. toolnbars).

And in cases where the icon isn't descriptive enough on it's own ...

I would argue icons are always not universally descriptive. It's not only about the ability to understand what an icon represent. It's also about differenc cultural beackgrounds, where the same icon may have a totally different meaning depending on the local culture. In many cases, icon-only is just not understandable by everyone.

Regarding the pattern icon + visible text, I'd like to remind there's already several places in the dropdowns where this pattern is in use. However, the placement and also the purpose of the icons is inconsistent. I'm not sure that helps the overall UI clarity. Rather, I'd think it adds a lot of cognitive load. See the screenshot below.

icons in dropdown

@ciampo
Copy link
Contributor Author

ciampo commented Nov 20, 2023

@afercia :

Will it still use a element though?
can you please point me to the relevant discussion and PR (if any)?

No "relevant" discussion about it.

The new version that is being worked on at the moment uses divs with roles menuitemradio and menuitemcheckbox (no <button />s), plus the aria-checked attribute. The implementation itself comes from ariakit, and you can take a look at our WIP on Storybook (there are dedicated examples for checkboxes and radios).

For accessibility reasons, visible text is always preferable.
...
I would argue icons are always not universally descriptive. [...] In many cases, icon-only is just not understandable by everyone.

I'll let @jameskoster reply to this point in a more complete manner, but in the meantime, I wanted to confirm these points were definitely taken into account, and that's why we've been considering text-only (no icon) as the safest, most clear way to represent radio/checkbox menu items.

For (the few cases) where designers may consider using icons, we adopt a solution similar to icon-toggles instead (example), of course always labelling icons properly (and adding tooltips).

Regarding the pattern icon + visible text, I'd like to remind there's already several places in the dropdowns where this pattern is in use. However, the placement and also the purpose of the icons is inconsistent. I'm not sure that helps the overall UI clarity. Rather, I'd think it adds a lot of cognitive load. See the screenshot below.

What you're flagging is true, and in fact one of the main goals of this new design spec is exactly to bring consistency in those UIs, especially around how icons are used and where they are displayed. So what you're flagging

@jameskoster
Copy link
Contributor

Rather, I'd think it adds a lot of cognitive load. See the screenshot below.

As @ciampo mentioned a big motivator for the spec was to tidy up the inconsistencies you've surfaced while incorporating new elements (like the chevronRight icon applied to flyout menu items).

To ensure reasonable spacing and alignment, (and to eliminate cognitive load when check/radio + decorative icon appear adjacent), it became clear that selection indicators and decorative icons should be mutually exclusive within a group. This produces satisfactory results in all applications with the (arguable) exception of the alignment (and similar) controls in the toolbar.

A version with smaller radios/checks was considered, but as you can see in the example below it doesn't work well in situations where a group contains selectable items without a decorative icon:

Selectability

We can potentially revisit such exceptions later, but it's quite tricky to come up with a pattern that intuitively supports both checkbox and radio applications without violating the alignment and spacing principles.

In the meantime those menus can continue to use the current component, be updated to use the new spec, or go icon-only.

@afercia
Copy link
Contributor

afercia commented Nov 27, 2023

@jameskoster thanks for the clarification.

if selectable items with icons were supported, the following could result, which is quite clumsy in appearance:

I'd agree a selectbale / checked indicaiton plus the icon would be clumsy. If that means dropdowns will not use icons any lonver in favor of visible text, that's a win-win.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants