nextcloud-vue icon indicating copy to clipboard operation
nextcloud-vue copied to clipboard

fix(NcActions): fix LI roles

Open ShGKme opened this issue 1 year ago • 0 comments

☑️ Resolves

Currently the menu in NcActions has the following structure:

<ul role="POPUP_ROLE">
  <li role="LI_ROLE" />
</ul>

POPUP_ROLE can be - menu | dialog | tooltip or nothing (=list) in navigation:

  • When it is a navigation, native roles works fine
  • When it is a menu, <li> has role presentation, because there are menuitem-s inside.
  • When it is dialog or tooltip, <li> had native listitem role because there is a list in the popup.

The problem is that in the last case <UL> is at the same time:

  • A list items' container - should have list role
  • A popup's container - should have dialog or tooltip role.

But it is not possible to have 2 roles. So, when it is a dialog or tooltip, list items are not in a list.

A proper solution should have been to wrap UL into another element.

<div role="POPUP_ROLE">
  <ul>
    <li />
  </ul>
</div>

However, we decided that this is too risky change of the structure for apps and testing.

Temporal solution - remove listitem role in dialogs and tooltips.

🖼️ Screenshots

Example 🏚️ Before 🏡 After
image image image

🚧 Tasks

  • [x] Better store HTML roles for different types of menu
  • [x] Provide NcActions context with as menu type and LI role
    • Relaces existing isSemanticMenu to provide more important data
  • [x] Add a composable to use this context
    • Replaces existing inject
  • [x] Set list item role on all action items, except NcActionSeparator (it is always separator)

P.S.

I haven't used existing mixins (actionText, actionGlobal) because they are not used on all the action items.

I haven't created a new mixin, because I decided, that we should move to composables.

🏁 Checklist

  • [ ] ⛑️ Tests are included or are not applicable
  • [x] 📘 Component documentation has been extended, updated or is not applicable

ShGKme avatar Feb 01 '24 22:02 ShGKme