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

[stable28] fix(menubar): menubar and table menus a11y #5330

Merged
merged 12 commits into from Feb 1, 2024

Conversation

backportbot[bot]
Copy link

@backportbot backportbot bot commented Feb 1, 2024

Backport of PR #5218

- Migrate from deprecated slot syntax
- Use a11y attrs from `NcPopover` on a custom button

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
All these roles are correctly set by `NcAction*` components when needed.
Setting these roles in mixins and menu bar puts them on wrong elements.

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
- It must have string `"true"/"menu"` value
- It must be placed on the `button` itself
- It is already correctly set by `NcActions` and `NcPopover`

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
- `aria-activedescendant` should identify a visually focused element when the real focus remains on this element.
  In the current implementation it identified selected element (even when the menu is closed), not the focused.
- `aria-activedescendant` is not needed because NcAction has actual focus anyway.
  In case it will be actually needed, it should be implemented in the `NcActions`

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
It has the same styles and correct a11y attributes.

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
- `aria-pressed` is not valid for a menu trigger button
- As alternative solution - show that there is a selected value directly in the text
- Remove incorrect prop for NcActions

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
- Since `@nextcloud/vue@8.6.0` correct attributes are covered by `NcAction*` and `NcButton` components, including fixes:
  - Attribute should display not only active `attr="true"` state, but also non-active `attr="false"`
  - It should be `aria-pressed` for buttons and `aria-checked` for menu items instead of `aria-selected`
- Set correct `type` and active state
  - `type="radio"` for a list of options like `Heading`
  - `type="checkbox"` for toggle buttons like `Bold`
  - `type="button"` for general buttons widhout active state like `Undo`

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
- ActionSingle was used in 2 places:
  1. As a single button in the menu
  2. As an item in NcActions
- NcActions doesn't fully support non-direct NcAction* children
- Move NcActionButton usage from ActionSingle to a new component
- This new component is named NcActionButton so that NcActions will consider it to be a valid NcAction* component

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
- Separates different parts visually
- Required for a11y to group radio button in the menu

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
…NcActions

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@backportbot backportbot bot added bug Something isn't working 3. to review accessibility labels Feb 1, 2024
@backportbot backportbot bot added this to the Nextcloud 28.0.3 milestone Feb 1, 2024
@juliushaertl juliushaertl merged commit e4c8c73 into stable28 Feb 1, 2024
53 checks passed
@juliushaertl juliushaertl deleted the backport/5218/stable28 branch February 1, 2024 12:18
@skjnldsv skjnldsv mentioned this pull request Feb 14, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants