Bug - [Drawer] - [examples give a11y errors with eslint a11y plugin]

[KKoukiou]

error tabIndex should only be declared on interactive elements jsx-a11y/no-noninteractive-tabindex

tabIndex used as here https://patternfly-react-v5.surge.sh/components/drawer/#basic

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[thatblindgeye]

In this case that tabindex is a sort of necessary "evil". When the drawer opens we need to move focus into it, and the text in that example is the first thing in the drawer. Placing it on the close button would be a possibility (no need for a tabindex on that div, the button is interactive), but that comes after the header content and might be missed.

We could update the example so that the close button comes first, just wondering if it would always makes sense to have those actions before the header content (if there's a kebab dropdown for example in that actions container). @mcoker was the intent behind Drawer actions that any action could be placed there, or only a close button should be rendered there?

[mcoker]

@thatblindgeye as far as the design intent, I'm not sure. Looking at the drawer design guidelines, it looks like there is also a kebab, but that's for a notification drawer. Going off of that, it seems like the drawer would also allow for additional actions? wdyt @andrew-ronaldson @lboehling?

There is no gap on <DrawerActions> (or a margin/spacer on its children), so if you did put multiple actions there currently, there would be no space between them. I thought the lack of a spacer might imply you shouldn't use multiple actions, but I noticed there is no spacer on notification drawer panel actions, either... even though the design guidelines (and our examples) have multiple actions in the notification drawer (I filed a bug here - https://github.com/patternfly/patternfly/issues/7366)

That said, the actions element is also manually positioned in the drawer panel's grid layout, so you could move it as the first thing in the drawer panel and it wouldn't impact the layout. <CardHeader actions={...}> is setup basically the same way, and card actions are the first thing rendered in <CardHeader>.

[thatblindgeye]

Depending on whether the intent is multiple actions could be passed to drawer actions or not, I feel like renaming that wrapper at some point in the future would make sense (if it's only meant to contain a close button, then having it named as a general "actions" wrapper is misleading).

If only a close button is meant to be placed there, then updating examples so that comes before anything else (similar to Modal) may make sense. Otherwise if other actions are valid, then I'd wonder if it'd be confusing/not a great experience if say, there's a kebab for additional actions that gets focused before even the drawer panel header ("Additional [drawer name] actions, button collapsed" being announced before "[drawer name/description]" for example; though if there is no drawer panel header then that's really the only focus order that makes sense).

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[thatblindgeye]

For now let's update any of these Drawer examples setting a tabindex so that the tabindex is set to -1 on any non-interactive element. That should still allow auto-focusing that content when the drawer opens without it being actually tabbable manually.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.