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

Element index view menus #11915

Merged
merged 9 commits into from
Sep 13, 2022
Merged

Conversation

brandonkelly
Copy link
Member

@brandonkelly brandonkelly commented Sep 11, 2022

Description

Adds a “View” menu to element index toolbars, with “Sort by” and “Table Columns” settings:

The new View menu for an entry index, showing that the view is configured to sort entries by their post date in descending order, and there are six table columns selected. There are “Use defaults” and “Close” buttons at the bottom of the menu.

Moving the sort options into the menu and splitting them into two separate select inputs resolves an accessibility issue (DEV-434 – listboxes have no provision for allowing multiple selections but restricted to disparate sub-sets of options within one control).

The Table Columns setting gives users the ability to customize the table columns should be visible, without affecting anyone else.

View menu selections will apply to the currently-selected top-level source and all its descendants. For example, if changes are made to a volume source on the Assets index, the same settings will apply to each of the volume’s subfolders’ sources (and vise-versa).

Note
There’s a security concern worth mentioning here: the Table Columns setting could give users the ability to see table columns that contain privileged info.

That has been mitigated by #11913 though. Now element index tables will hide the columns for any custom fields that aren’t visible to the current user per the field layout’s user conditions (either on the custom field or its tab).

Related issues

@linear
Copy link

linear bot commented Sep 11, 2022

DEV-994 Element index view menus

Element index sort options should be moved into a “View” disclosure menu, where we could also put additional options such as the ability to choose which table columns are visible (per user + source, like the sort setting).

@gcamacho079
Copy link
Contributor

@brandonkelly just completed a full audit on the updates! Here are a few a11y issues I noted:

  • Sort attribute and direction select elements do not have visible or programmatic labels. (Per WCAG 2.1 AA, all field labels should be visible.)
  • Currently "Sort by" and "Table Columns" text use label elements. However, these grouped fields should be wrapped in a fieldset element, where “Sort by” and “Table Columns” are the legend elements
  • When activating the "Use defaults" button, focus should return to the "View" button
  • At 200% text zoom on Firefox, the sort attribute and direction select elements become narrow and the selected values aren’t easy to read; these should probably stack by default or at a certain breakpoint to allow for more of the selected value to be read.
  • At a 320px wide screen size, the disclosure menu is entirely offscreen (see related issue DEV-959)

@linear
Copy link

linear bot commented Sep 12, 2022

DEV-722 The "Sort by" custom select introduces parsing issues caused by multiple options having identical IDs

Page Area:

"Sort by" custom select, [data-extra] options

Issue Description:

All options with attribute [data-extra] have identical IDs, causing parsing issues for screen reader users

Action:

Ensure all options have unique IDs

Screenshot/Code Snippet:

Screenshot showing many options with the same IDs

Screen Shot 2022-06-29 at 2.48.54 PM.png

Resolves CMS-250

@brandonkelly
Copy link
Member Author

Thanks @gcamacho079!

  • Sort attribute and direction select elements do not have visible or programmatic labels. (Per WCAG 2.1 AA, all field labels should be visible.)

Added aria-label="Sort attribute" and aria-label="Sort direction".

  • Currently "Sort by" and "Table Columns" text use label elements. However, these grouped fields should be wrapped in a fieldset element, where “Sort by” and “Table Columns” are the legend elements

Done.

  • When activating the "Use defaults" button, focus should return to the "View" button

I’ve tweaked the behavior so the menu no longer closes when you press that button. The button is still removed, though, so I’m moving the focus to the “Close” button instead.

  • At 200% text zoom on Firefox, the sort attribute and direction select elements become narrow and the selected values aren’t easy to read; these should probably stack by default or at a certain breakpoint to allow for more of the selected value to be read.

I switched the direction to a listbox instead of a dropdown, which gives the attribute dropdown a bit more room.

The “Sort by” fieldset, showing that it contains an attribute dropdown and a listbox with “Sort ascending” and “Sort descending” buttons.

  • At a 320px wide screen size, the disclosure menu is entirely offscreen (see related issue DEV-959)

Added a media query so any viewport under 400px wide will now stack those “meta” field labels + inputs vertically instead of side-by-side

CleanShot 2022-09-12 at 17 47 26@2x

@gcamacho079
Copy link
Contributor

@brandonkelly love the button group you introduced to save space. However I'm taking a look at the Craft.Listbox component and I think we should consider deprecating and replacing it as it doesn't quite match with other well-tested, accessible patterns. I can branch off of here to create an "exclusive button group" component that we can use in various places (such as for view mode on asset index pages, and device type in the Preview modal).

Added a media query so any viewport under 400px wide will now stack those “meta” field labels + inputs vertically instead of side-by-side

The stacking looks great! However I'm still getting a horizontal scrollbar at desktop screen sizes though, and when text size is increased to 200% on Firefox or text-spacing is applied.

At 320px, here's the issue that's popping up with the disclosure positioning:
The menu that pops up under the View button is positioned against the right edge of the button, and so only a portion of the contents are visible.

# Conflicts:
#	src/web/assets/cp/dist/cp.js
#	src/web/assets/cp/dist/cp.js.map
#	src/web/assets/cp/dist/css/cp.css
#	src/web/assets/cp/dist/css/cp.css.map
@brandonkelly
Copy link
Member Author

Doh, should have tested on the Assets index page. That menu-out-of-viewport bug is fixed now.

However I'm taking a look at the Craft.Listbox component and I think we should consider deprecating and replacing it as it doesn't quite match with other well-tested, accessible patterns. I can branch off of here to create an "exclusive button group" component that we can use in various places (such as for view mode on asset index pages, and device type in the Preview modal).

That component is pretty fresh. In core it’s only used for the “Fields”/“UI Elements” toggle within field layout designers, and now this. Not sure if other plugins are using it yet, though. Depending on what you’d want to change, it might make sense to just update the existing component, either for 4.x or just push off to Craft 5.

@brandonkelly brandonkelly merged commit 5230de7 into 4.3 Sep 13, 2022
@brandonkelly brandonkelly deleted the feature/dev-994-element-index-view-menus branch September 13, 2022 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants