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

native icons decorative images guidance #180

Merged
merged 5 commits into from
Jan 8, 2025

Conversation

danbivins
Copy link
Collaborator

Only covering decorative image/icons for this.

@danbivins danbivins requested review from marshcooke and jzoo January 7, 2025 21:08
marshcooke
marshcooke previously approved these changes Jan 7, 2025
Copy link
Collaborator

@marshcooke marshcooke left a comment

Choose a reason for hiding this comment

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

I got mentally hung up on 2 things:

  • The "keyboard" reproduction steps that mention "Screenreader ignores.." since KB users might not also be using a screen reader (on lines 7-9).
  • The comma used between "adjacent" and "text" in line 45 - should it be removed or am I overthinking it?

And I know we have a lot of these throughout the site, so it shouldn't warrant a change but at least wanted to mention it:

  • The double space between sentences on line 36.

Otherwise, this all looks great to me! I appreciate you putting this resource together!!

Copy link
Collaborator

@jzoo jzoo left a comment

Choose a reason for hiding this comment

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

we do not have Gherkin or WCAG for native right now so maybe comment out WCAG for now and for Gherkin we can just watch to see how it turns out as it is new. We are working on adding Gherkin for native so this will be a good test/start to see how it looks and turns out.

_checklist-native/image-decorative.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@marshcooke marshcooke left a comment

Choose a reason for hiding this comment

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

Nice!

@danbivins
Copy link
Collaborator Author

@jzoo updated with your helpful feedback

Copy link
Collaborator

@jzoo jzoo left a comment

Choose a reason for hiding this comment

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

awesome, looks good

@danbivins danbivins merged commit b2be462 into main Jan 8, 2025
3 checks passed
@danbivins danbivins deleted the dbivins-native-icons-images-guidance branch January 8, 2025 20:39
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