-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
MDN React Tutorials #6: Accessibility & Resources #31139
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A two-for-one special to close us out, @chrisdavidmills! Mostly some little line edits, but I called out some notable stuff.
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work as always @mxmason! I've worked through the whole tutorial and all is working well. I've got some comments for you here, but nothing too major.
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Chris Mills <[email protected]>
files/en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_resources/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to you, @chrisdavidmills!
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mxmason lovely work! Just a few more changes/suggestions for you to consider. Very close now!
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Chris Mills <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to you @chrisdavidmills! As I said, I'm on my phone so pardon the mess – I just wanted to get this back to you ASAP.
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
...en-us/learn/tools_and_testing/client-side_javascript_frameworks/react_accessibility/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mxmason OK, changes made, and approving!
Have another look at the updated usePrevious()
description, and let me know if you are happy. I mostly used your wording.
This looks great, @chrisdavidmills! I'm good to move on with this if you are. I assume that mdn/todo-react#97 blocks this series of content PRs, so I'll do my best to get to your notes today. I'm also fine with merging and releasing this content, if you'd prefer. And, of course, I'm happy to do one final sweep over all this content after we merge, just to make sure it's all tidy. I'm just excited to get this out! 😁 |
Faaaantastic. Ideally, I'd like the demo repo to be updated before we merge the content, but I don't see that as an absolute dealbreaker as long as it is merged soon after. As you've seen on the other repo, I'm having trouble there because I don't have the privs to merge on that repo. If I don't find someone who does in the next 30 mins or so, I'm going to go ahead anyway and merge the content. |
@mxmason And with that, your work is all merged, and the job is complete. It should go live on MDN in about 24 hours, possibly sooner. As soon as that happens, I'll give everything a read-through and get ready to file a PR to fix any minor bits we may have missed. Great work though! It has been a pleasure working with you again. I hope you have a restful festive period. |
@mxmason your updates are live! I'll give them a good read through now, and see if I can spot any typos. Let me know if you spot any, and I'll add them into a PR of fixes. |
* chore: minor line edits * chore: moar line edits * chore: add note on focus-visible * chore: yet another line edit * chore: update style section * chore: clean up whitespace * chore: add useReducer section * chore: clean up client side router section * chore: fix typo * chore: update dev tools reference link * Minor grammar fixes * small tweaks * chore: apply suggestions from code review Co-authored-by: Chris Mills <[email protected]> * chore: add useRef explainer * chore: rewrite ref/effect * chore: remove mention of create-react-app from testing section * chore: update note on focus-visible * chore: add usePrevious explainer * chore: dedupe * chore: more little rewrites * chore: simplify length check * tweaks to :focus-visible explanation * minor language tweak * chore: apply suggestions from code review Co-authored-by: Chris Mills <[email protected]> * usePrevious() description update * Remove "generally" --------- Co-authored-by: Chris Mills <[email protected]>
Description
Following #31093, as part of mdn/mdn#474, this PR makes some modifications to the sixth and seventh pages in our "Getting started with React" tutorial.
Do not merge this until #31093 has been merged and you have updated this PR with mdn main and checked for breakages.