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.

Sign up for GitHub

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

Jump to bottom

Editorial: Add landmark language to definitions #1060

Merged
merged 3 commits into from
Oct 10, 2019
Merged

Editorial: Add landmark language to definitions #1060

merged 3 commits into from
Oct 10, 2019

Conversation

jnurthen
Copy link
Member

@jnurthen jnurthen commented Sep 26, 2019 •
edited by pr-preview bot
Loading

resolves #84

Preview | Diff

@jnurthen jnurthen requested a review from joanmarie September 26, 2019 22:52
@carmacleod
Copy link
Contributor

@jnurthen Your changes look good.

Wondering, though, why contentinfo is "A large perceivable landmark" when in fact it is no larger or more perceivable than a banner or any other landmark. Can we delete the 2 words "large perceivable"?

Similarly, for region: "A perceivable landmark", can we delete the word "perceivable"?

@jnurthen
Copy link
Member Author

@carmacleod happy to make those changes. I was simply making the minimum changes possible to resolve the comment.

@carmacleod
Copy link
Contributor

@jnurthen Understood. It just struck me as "off" when reviewing. :)

@jnurthen jnurthen added the editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo label Sep 27, 2019
@carmacleod
Copy link
Contributor

Thanks!

joanmarie
joanmarie reviewed Oct 2, 2019
View reviewed changes
joanmarie
joanmarie reviewed Oct 2, 2019
View reviewed changes
joanmarie
joanmarie reviewed Oct 2, 2019
View reviewed changes
joanmarie
joanmarie reviewed Oct 2, 2019
View reviewed changes
joanmarie
joanmarie reviewed Oct 2, 2019
View reviewed changes
joanmarie
joanmarie reviewed Oct 2, 2019
View reviewed changes
joanmarie
joanmarie previously requested changes Oct 2, 2019
View reviewed changes
Copy link
Contributor

@joanmarie joanmarie left a comment

Choose a reason for hiding this comment

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

Unless there's a good reason not to, let's nuke "region" from these statements.

@jnurthen jnurthen requested a review from joanmarie October 2, 2019 17:42
scottaohara
scottaohara approved these changes Oct 8, 2019
View reviewed changes
Copy link
Member

@scottaohara scottaohara left a comment

Choose a reason for hiding this comment

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

looks good to me and all regions have been removed

@jnurthen jnurthen merged commit 4228b98 into master Oct 10, 2019
jnurthen added a commit that referenced this pull request Oct 10, 2019
@jnurthen
Editorial: Add landmark term 2 definitions(#1060)
59d325f
@jnurthen jnurthen deleted the Editorial-Landmarks branch October 14, 2019 21:16
carmacleod pushed a commit that referenced this pull request May 7, 2020
@jnurthen @carmacleod
Editorial: Add landmark term to all landmark definitions & make consi…
22703a0 
…stent (#1060)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottaohara scottaohara scottaohara approved these changes

@joanmarie joanmarie Awaiting requested review from joanmarie

Assignees
No one assigned
Labels
editorial a change to an example, note, spelling, grammar, or is related to publishing or the repo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

WAI-ARIA 1.1: Editorial issue re. definition for landmark roles
4 participants
@jnurthen @carmacleod @joanmarie @scottaohara