Fix menu width operator-precedence bug and remove invalid Tailwind z-100 class #667
Conversation
This file contains hidden or 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
…100 class PR title Fix menu width operator-precedence bug and remove invalid Tailwind z-100 class PR description This PR fixes two bugs in apps/web/components/menu.tsx: 1. Fix operator-precedence bug when calculating menu width - Problem: `const menuWidth = expandedView || isCollapsing ? 600 : isHovered ? 160 : 56` relied on implicit precedence and could evaluate unexpectedly. - Fix: wrap the first condition in parentheses so the expression is evaluated as intended: `const menuWidth = (expandedView || isCollapsing) ? 600 : (isHovered ? 160 : 56)` 2. Remove invalid Tailwind class `z-100` and use the existing arbitrary z-index variable - Problem: The code contained a raw `z-100` class which is not part of default Tailwind utilities (and will have no effect unless added to config). - Fix: Remove the literal `z-100` from the trigger wrapper and rely on the `mobileZIndex` value that uses arbitrary values (`z-[70]` / `z-[100]`). This ensures the trigger gets the intended z-index without requiring custom Tailwind config. Files changed - apps/web/components/menu.tsx Why - The operator-precedence fix prevents unexpected menu width behaviour when expanding/collapsing and when hovering. - Removing `z-100` avoids relying on non-standard Tailwind classes and reduces surprising styling bugs on mobile (trigger might be hidden behind other elements). How to test - Desktop: - Hover the left menu to confirm it expands to the hover width (160). - Click items that open expanded views and confirm the menu expands to the expanded width (600) and collapses correctly. - Mobile: - Open the mobile menu trigger and confirm the floating trigger is positioned above other UI (z-index behavior). - Open/close the drawer and verify no z-index regressions cause the trigger or drawer to be hidden by other elements. - Lint/TypeScript: - Run the project's lint/type-check/build to confirm there are no type or lint errors. Notes / follow-ups - The file contains an MCPIcon SVG with truncated path data in the snippet I edited; if the real SVG in repo has full paths, ignore this. If the repo contains literal "[...]" or truncated path data, we should replace that with the full path data for the icon to render correctly. - Optionally, if you want to keep a `z-100` utility, add it to tailwind.config.js instead of relying on non-standard classes. If you want, I can open a branch and push this change as a PR for you; tell me the target branch name and which branch to base it on.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This PR fixes two bugs in apps/web/components/menu.tsx:
const menuWidth = expandedView || isCollapsing ? 600 : isHovered ? 160 : 56relied on implicit precedence and could evaluate unexpectedly.const menuWidth = (expandedView || isCollapsing) ? 600 : (isHovered ? 160 : 56)z-100and use the existing arbitrary z-index variablez-100class which is not part of default Tailwind utilities (and will have no effect unless added to config).z-100from the trigger wrapper and rely on themobileZIndexvalue that uses arbitrary values (z-[70]/z-[100]). This ensures the trigger gets the intended z-index without requiring custom Tailwind config.Files changed
Why
z-100avoids relying on non-standard Tailwind classes and reduces surprising styling bugs on mobile (trigger might be hidden behind other elements).