Skip to content

Update dialog.xml #240

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

Closed
wants to merge 2 commits into from
Closed

Update dialog.xml #240

wants to merge 2 commits into from

Conversation

mattbrundage
Copy link
Contributor

Diction regarding the focus state

Diction regarding the focus state
@scottgonzalez
Copy link
Member

Can you explain why this is better?

@dylanb
Copy link

dylanb commented Nov 19, 2014

They seem different, is the new verbiage accurate? Does the dialog in fact prevent (for example) screen reader navigation beyond the boundaries of the dialog?

@mattbrundage
Copy link
Contributor Author

It was an edit solely for clarity -- not to change meaning. Reasoning: I initially read the phrase "tabbing cycles" as a compound noun, and the word "focus" as a verb.

@dylanb : I can't speak specifically to screen reader navigation, but dialog does in fact successfully prevent traditional keyboard navigation from focusing on elements beyond its boundaries.

@dylanb
Copy link

dylanb commented Nov 19, 2014

perhaps "keyboard navigation using the 'tab' key, causes the focus to cycle amongst the focusable elements in the dialog..."?

@mattbrundage
Copy link
Contributor Author

Yeah, that'll work too.

@dylanb
Copy link

dylanb commented Nov 19, 2014

@scottgonzalez you ok with that?

@scottgonzalez
Copy link
Member

yup

@dylanb
Copy link

dylanb commented Nov 20, 2014

@mattbrundage you want to make that change to your PR?

Modified diction, as suggested by @dylanb
@agcolom
Copy link
Member

agcolom commented Dec 14, 2014

@scottgonzalez Are you happy with the PR?

@scottgonzalez
Copy link
Member

Yes, this looks good. Can you please sign the CLA?

@mattbrundage
Copy link
Contributor Author

Signed.

jzaefferer pushed a commit that referenced this pull request Jan 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants