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

chore: spiff up the keyboard navigation codelab. #1671

Merged
merged 16 commits into from
May 19, 2023

Conversation

jubishop
Copy link
Contributor

@jubishop jubishop commented Apr 21, 2023

follows standards/patterns using in custom renderer/generator.

fixes: #1657

@jubishop jubishop requested a review from a team as a code owner April 21, 2023 20:41
@jubishop jubishop requested review from cpcallen and removed request for a team April 21, 2023 20:41
@jubishop jubishop changed the title Keyboard nav chore: spiff up the keyboard navigation codelab. Apr 21, 2023
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

Overall looks like a good set of improvements. I have two overall comments:

  • For clarity, I think it would be best to word all instructions in the imperative, without using "you"—so, "now do x", rather than "now you will do x". Keep use of "you" for observations that are not instructions. (I've noted a number of these individually, but I recommend giving the whole text a copy-edit with this suggestion in mind.)
  • I think if we're changing renderers then we should also re-create the images, but feel free to leave both those for a subsequent PR if you wish (i.e. reverting the render change for now); the other changes here seem worthwhile on their own.

@jubishop
Copy link
Contributor Author

jubishop commented May 5, 2023

thanks @cpcallen , I just went back to geras for now.

@jubishop
Copy link
Contributor Author

jubishop commented May 5, 2023

all language should be updated now.

@cpcallen
Copy link
Contributor

@jubishop: I don't believe you have commit access, so let me know if you're ready for this to go in.

@jubishop
Copy link
Contributor Author

yes please, thanks!

@cpcallen cpcallen merged commit 7ce5662 into google:master May 19, 2023
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.

Improvements to Keyboard Navigation
2 participants