Uh oh!
There was an error while loading. Please reload this page.
Fixed: Ensure Header component uses proper semantic header HTML tag - #3533
Conversation
🦋 Changeset detectedLatest commit: 63b936a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
amarmanhala
commented
Jul 18, 2023
Hi @joshblack, This is my PR. Please can you guide me if there is anything that I missed or any advice? Thanks |
joshblack
left a comment
There was a problem hiding this comment.
Looks great! Thanks for doing this 🥳
joshblack
commented
Jul 18, 2023
For the test failure, I think all that's left to do is update the snapshots for the header tests! You can do this by running the following command: npm test -- -uAnd then including the snapshots in a commit and pushing them up to this Pull Request. Hope that makes sense, let me know if you have any questions! |
amarmanhala
commented
Jul 19, 2023
Thank you @joshblack for your guidance 🤗. I have committed test snapshots now. Hopefully, it works now. |
joshblack
commented
Jul 19, 2023
@amarmanhala seems like just one final change, in the test file for it('renders a <div> and <a>', () => {
- expect(render(<Header />).type).toEqual('div')+ expect(render(<Header />).type).toEqual('header')
expect(render(<Header.Link />).type).toEqual('a')
})After that you should be good to go! 🥳 |
amarmanhala
commented
Jul 20, 2023
Thank you @joshblack for your guidance. |
joshblack
commented
Jul 20, 2023
Thanks so much for your contribution @amarmanhala! 🙌 🥳 |
Changed div tag to HTML semantic element for Header component
Issue #2870
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.