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

Links components (e.g. 'Footer links') can not be tabbed to #6305

Merged
merged 10 commits into from
Sep 15, 2019
Merged

Links components (e.g. 'Footer links') can not be tabbed to #6305

merged 10 commits into from
Sep 15, 2019

Conversation

BatJan
Copy link
Contributor

@BatJan BatJan commented Sep 8, 2019

Prerequisites

  • I have added steps to test this contribution in the description below

This fixes issue no. 34 from #5277

Description

It appears that since the release of v8 some of the original directive attributes for "openUrl", "removeUrl" and "editUrl" have been removed - At least they don't return any values anymore, which make the <a> act like a placeholder link, which will not receive keyboard focus until there is a href attribute present.

I had a brief chat with @abjerner about this since the history revealed that he introduced the mentioned options with the release of Umbraco 7.12.0 and he explained the intention so instead of removing the <a> I have added a check to decide whether the control should be wrapped in <a> or <button>. If the attributes mentioned are falsy we will render a <button>.

This means that the "Edit", "Open" and "Remove" options are now keyboard accessible again and if the mentioned options are added at a later stage the <a> will be used instead since it then should act like a link 👍

The aria attributes aria-haspopup and aria-expanded have been added and the umbnodepreview.directive.js has been updated with some options to add these values.

For the "Open" option I have only added the aria-haspopup attribute for now since the way the infinifte open/close works makes it harder to set the proper true/false value for aria-expanded so will need to have another think about the approach for dealing with this issue.

I realized that I was using the aria-haspopup/aria-extended in the wrong context here. It's meant for exposing menus and not dialogs.

The aria-haspopup attribute was updated so it can take the value "dialog" but support is probably not that widespread among screen readers yet. When testing with NVDA nothing is announced so I think it should be ok to leave it in so screen readers will pick it up once they have support for it.

Before
edit-options-before

After
edit-options-after

@nul800sebastiaan nul800sebastiaan merged commit de656ac into umbraco:v8/dev Sep 15, 2019
@nul800sebastiaan
Copy link
Member

Nice one, thanks @BatJan ! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants