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 lang="en" to relevant snippets in build-system/ #34757

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

caroqliu
Copy link
Contributor

@caroqliu caroqliu commented Jun 8, 2021

This PR is a partial copy of #31208, which adds lang="en" to relevant code snippets in this codebase. Instead of copying the PR file-for-file, I decided to break this change up into a few root directories for ease of review and less likelihood of falling behind to merge conflicts.

Original PR description:

Suggested changes and additions based on the TetraLogical accessibility review commissioned by @nainar / @caroqliu

x-ref ampproject/amp.dev#4972

/cc @TetraLogicalHelpdesk

@amp-owners-bot amp-owners-bot bot requested a review from estherkim June 8, 2021 17:37
@amp-owners-bot
Copy link

amp-owners-bot bot commented Jun 8, 2021

Hey @alanorozco! These files were changed:

build-system/server/app-index/amphtml-helpers.js

Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

These files are used by AMP developers working on the library itself?

@caroqliu
Copy link
Contributor Author

caroqliu commented Jun 8, 2021

These files are used by AMP developers working on the library itself?

Yes. In particular, the templates for the amp make-extension task are helpful to update as they create sample documents that people may use as exemplary. I don't feel strongly about changing the other two files. LMK if I should revert those.

@kristoferbaxter
Copy link
Contributor

Changing them all makes sense, just wanted to make sure I understood where the value was.

Thanks!

@caroqliu caroqliu merged commit 16fe880 into ampproject:main Jun 9, 2021
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.

2 participants