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

add an aria-label to avoid the dialog content being read twice #38

Merged
merged 6 commits into from
Aug 8, 2018

Conversation

vlukashov
Copy link
Contributor

@vlukashov vlukashov commented Jul 23, 2018

Without an aria-label screen readers (NVDA, JAWS, VoiceOver) read the dialog content twice: once as a title and another time as the content.

Depends on vaadin/vaadin-dialog#91
Fixes #29


This change is Reviewable

Viktor Lukashov added 3 commits July 23, 2018 16:56
Without an `aria-label` screen readers (NVDA, JAWS, VoiceOver) read the dialog content twice: once as a title and another time as the content.

Depends on vaadin/vaadin-dialog#91
Fixes #29
@alvarezguille
Copy link
Member


bower.json, line 31 at r1 (raw file):

2.1.0-beta2

Let's not depend on new minor version unless a stable version is released before V11beta

Copy link
Member

@alvarezguille alvarezguille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r1.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @vlukashov)

Copy link

@alexberazouski alexberazouski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @alvarezguille and @vlukashov)


src/vaadin-confirm-dialog.html, line 227 at r2 (raw file):

        _getAriaLabel(header) {
          return header || 'confirmation';

There is an option to set a custom header element into slot. Should we support this case as well?

Copy link

@alexberazouski alexberazouski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r3.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link

@alexberazouski alexberazouski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@alexberazouski alexberazouski merged commit 536b5b0 into master Aug 8, 2018
@alexberazouski alexberazouski deleted the fix/29-dialog-label-for-screen-readers branch August 8, 2018 12:36
alexberazouski pushed a commit that referenced this pull request Aug 30, 2018
* add an aria-label to avoid the dialog content being read twice

Without an `aria-label` screen readers (NVDA, JAWS, VoiceOver) read the dialog content twice: once as a title and another time as the content.

Depends on vaadin/vaadin-dialog#91
Fixes #29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants