You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
Addresses (issue 977: Hover styling does not span the entire width of menu)[#977].
How
We add the tailwind classes w-full and justify-start (which translate to: width: 100%; and justify-content: start) to all links in the mobile menu. This expands the element to the full width of the mobile menu while keeping the text aligned correctly.
Screenshots
Before:
After:
Testing instructions:
View current state
First, go to cleanandgreenphilly.com
View the (live, production) website with screen dimensions wider than 640px, but narrow enough that the mobile (hamburger) menu is available.
Open the dropdown menu by clicking on the hamburger toggle in the nav bar
Observe that the button for the current page is highlighted green
Hover over any of the other buttons. The button should have a darker gray background than un-hovered buttons. The background should completely wrap the content text of the button, but not span the width of the menu as desired.
View proposed changes
Run code locally
Repeat steps 2 through 4 for the local version of the website
Hover over any of the non-active buttons (not highlighted with a green background.) The button should have a darker gray background as in production, but the background should fully extend to fill the mobile menu. The text should remain left-aligned (or right-aligned, if viewed with the browser set to a right-to-left language).
@millmason thank you for the detailed testing instructions. I just tested your PR. Gray hover styling now spans the mobile nav menu's entire width. Thanks!
By the way, I've changed this PR's base branch from main to staging. I'll update this repo's contributors guide to let folks know that generally they should submit PRs against staging branch.
@bacitracin I noticed that on our live website, the mobile nav menu has no hover styling on screens smaller than 640px. That behavior is unrelated to this PR. Do you know whether that's a bug, or if the menu is working as intended?
@CodeWritingCow Hmm at < 640px the mobile menu background color still changes on hover, from the page color to a brighter white. This is the same as at higher widths. So I'd say it's a bug, and the dark grey hover over each option is missing.
@bacitracin@CodeWritingCow I don't have write-access to the repo so I can't merge the branch -- should I ping someone to do so? (Not to rush, just making sure I'm not omitting a step!)
@CodeWritingCow Hmm at < 640px the mobile menu background color still changes on hover, from the page color to a brighter white. This is the same as at higher widths. So I'd say it's a bug, and the dark grey hover over each option is missing.
Just created bug ticket #1079 to track this issue!
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.
CodeWritingCow
requested review from
bacitracin
and removed request for
zigouras
What
Addresses (issue 977: Hover styling does not span the entire width of menu)[#977].
How
We add the tailwind classes
w-fullandjustify-start(which translate to:width: 100%;andjustify-content: start) to all links in the mobile menu. This expands the element to the full width of the mobile menu while keeping the text aligned correctly.Screenshots
Before:
After:
Testing instructions:
View current state
View proposed changes