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

Accessibility - fix keyboard focus for popups/dialogs (e.g. support), change secondary form button (e.g. cancel) style #5717

Closed
kaitlinnewson opened this issue Apr 3, 2019 · 7 comments · Fixed by #5903
Assignees
Labels
Feature: Accessibility Accessibility is the practice of making your websites usable by as many people as possible.
Milestone

Comments

@kaitlinnewson
Copy link
Contributor

Tested in 4.10 and 4.11 in both Scholars Portal and Harvard Dataverses, in Firefox and Chrome (OSX).

When opening an overlay, e.g. the Support form, the user's keyboard focus should go there. The overlay is automatically added to the end of the tab order, meaning the user has to tab through the whole page underneath before they can get to the overlay window.

@TaniaSchlatter
Copy link
Member

Hi @kaitlinnewson, thanks for opening this issue. We are working on improving accessibility when working on related functionality. Its helpful to raise the visibility of this and other accessibility issues. Have you come across any best practices for addressing accessible popups?

@pdurbin pdurbin added the Feature: Accessibility Accessibility is the practice of making your websites usable by as many people as possible. label Apr 3, 2019
@kaitlinnewson
Copy link
Contributor Author

Hi @TaniaSchlatter, this article from uxplanet outlines some best practices (both for ux and accessibility): https://uxplanet.org/best-practices-for-modals-overlays-dialog-windows-c00c66cddd8c

@TaniaSchlatter
Copy link
Member

I also found an accessibility plugin for Bootstrap that addresses popup focus. I am not sure if this might be a fit, but the team will review these references and make a plan. https://github.com/paypal/bootstrap-accessibility-plugin.

@mheppler
Copy link
Contributor

mheppler commented May 21, 2019

Was able to add a PrimeFaces p:focus component inside the h:form in the p:dialog for the Support popup, and added context attribute with the popup ID.

Screen Shot 2019-05-22 at 3 25 05 PM

In addition to making sure the first input of the form receives focus, we will be changing the "Cancel" btn styles to deemphasize the secondary action, by using the btn-link class from Bootstrap to make the button look like a text link. This same style change should be applied to confirmation dialog popups, as well as full page forms. We can also make sure the focus is properly applied to the first input in the full page forms as well.

Updated issue title to reflect full scope of the issue -- Accessibility - fix keyboard focus for popups/dialogs (e.g. support), change secondary form button (e.g. cancel) style

NOTE -- Redacted outdated and obfuscated checklist in an attempt to generate a clearer outline of all the moving parts in a comment on the pull request.

BUG1 -- the p:focus component does not seem to be able to apply the focus to all types of form components, including inputTextarea, tieredMenu, selectManyCheckbox, selectOneRadio, selectBooleanCheckbox, selectCheckboxMenu. This is a known issue with PrimeFaces and appears to have been improved in PrimeFaces 7.

Various components have been enhanced for accessibility related to keyboard support and screen readers.

BUG2 -- existing bug and confirmed in demo, the Save Changes btn in dataset-widgets.xhtml, "themeWidgetsForm" does nothing when clicking without making changes, and it would intermittently do nothing when using return key to submit.

@mheppler mheppler changed the title Accessibility - when opening an overlay (e.g. support), keyboard focus should go there. Accessibility - fix keyboard focus for popups/dialogs (e.g. support), change secondary form button (e.g. cancel) style May 28, 2019
mheppler added a commit that referenced this issue May 29, 2019
@scolapasta scolapasta added Medium and removed Small labels Jun 3, 2019
@mheppler mheppler mentioned this issue Jun 3, 2019
5 tasks
@pdurbin
Copy link
Member

pdurbin commented Jun 11, 2019

I just scrolled through the thousands of lines of code changes in pull request #5903 as of fd05813 and nothing jump out at me as being wrong or bad (the addition of webmanifest to web.xml gave me pause but the change is explained in the pull request) but this is obviously a lot of code and we will have to rely on testing to confirm that nothing is broken.

Toward that end @mheppler has provided a long and extremely detailed list of what to test at #5903 (comment)

When I say long and detailed, I really mean it. Here's a thumbnail of how long the list is:

Screen Shot 2019-06-11 at 12 04 11 PM

This strikes me as more than "Medium".

I went ahead and spun up the branch to get a sense of how the buttons look. Here are a couple examples. Note how "Cancel" is just a text link:

Screen Shot 2019-06-11 at 11 57 59 AM

Screen Shot 2019-06-11 at 11 58 16 AM

I think having the primary button be emphasized like this will increase the usability of Dataverse so I'm excited about this change! Developers, please note that this change has also been noted in the Style Guide in 62642ef.

@pdurbin pdurbin removed their assignment Jun 11, 2019
@kcondon
Copy link
Contributor

kcondon commented Jun 11, 2019

Issues so far:
A few false alarms due to cached css.
[ ] Reset/forgot password submit throws log error when email is valid form but fake. Does not display reset sent or status message. Works on other branch.
[ ] Dataset versions differences checkboxes missing when 3 versions exist

@mheppler
Copy link
Contributor

Reverted my passwordreset changes to resolve the issue @kcondon reported above. There is a lot going on with that page that needs to be address, and I have found a recently opened issue (#5462) that is the perfect place to park all these related UI/UX issues I discovered.

Found that the dataset versions tab issue reported above was due to the test dataset on dataverse-internal containing a previous version that was deaccessioned, which resulted in the View Differences btn and checkboxes not to be rendered.

Reported all of this in my really, really, really long comment in the PR, as well as in Slack, directly to Kevin. Should be good to re-QA, confirm and ship ⛵️.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Accessibility Accessibility is the practice of making your websites usable by as many people as possible.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants