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

docs: Fix broken link to aria-toggle-field-name #10246

Merged

Conversation

msomji
Copy link
Contributor

@msomji msomji commented Jan 22, 2020

Addresses #10243
docs: Fix broken link to aria-toggle-field-name.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

LGTM thanks @msomji!

Copy link
Member

@exterkamp exterkamp left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

The only hesitation I have with sending this through, is that everything in ...locales/.* should be machine edited in an i18n import (except en and en-XL). Really the only change needed is to lighthouse-core/audits/accessibility/aria-toggle-field-name.js, and then update the en and en-XL automatically with yarn i18n:collect-strings, then we will import the new links on the next auto-import. This same thing happened once with a PR not too long ago. It makes me want to lock down the .../locales/.* folder, but at the end of the day, these changes fix it faster than waiting for me to run the round trip scripts, so 🤷‍♂️ This is probably a fine way of updating them until the auto import comes in and no-op's it anyway.

@paulirish
Copy link
Member

@exterkamp can you file an issue to track the i18n process changes you're hinting at?

once that's sorted you can add land when green

@exterkamp
Copy link
Member

tracking in #10276

@devtools-bot devtools-bot merged commit 8d8a052 into GoogleChrome:master Jan 28, 2020
@msomji msomji deleted the docs/aria-toogle-field-name branch January 28, 2020 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants