[Issue #2440] Update Header.tsx for index link #2838
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andycochran
approved these changes
Nov 13, 2024
doug-s-nava
approved these changes
Nov 14, 2024
There was a problem hiding this comment.
✅ ✅ ✅ ✅
LGTM with a couple notes:
- as far as testing, in my recent PR that touches the nav, I just asserted on the href. Is there a reason that wouldn't work here? https://github.com/HHS/simpler-grants-gov/pull/2756/files#diff-6d000d543d1b71f0d9073671adc8ded767ef291901bd4e47e4c556ad1469d53e
- Note that I have managed to get my local app to always serve me the Spanish site, which means I'm directed to localhost:3000/es. In that case, I still get a link on the header. Something to either handle now or remember to think about when we implement the Spanish site
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2440
Time to review: 5 mins
Changes proposed
Add link to site name in the header. Removes link if you are on the homepage. I don't love the user experience of sites that don't do that with their logo.
I looked at testing the link change but couldn't get it working timeboxing myself vercel/next.js#42527 .