fix(web): Set DropdownMenu styles to use minWidth #17185
Conversation
WalkthroughThe pull request involves the deletion of the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
libs/island-ui/core/src/lib/DropdownMenu/DropdownMenu.css.ts (1)
5-5: Consider using theme spacing for minWidthWhile changing to
minWidthis good, consider using theme spacing values for better maintainability and consistency across the design system.- minWidth: 150, + minWidth: theme.spacing[19], // or appropriate theme spacing value
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
apps/web/components/Header/LoginButton.css.ts(0 hunks)apps/web/components/Header/LoginButton.tsx(0 hunks)libs/island-ui/core/src/lib/DropdownMenu/DropdownMenu.css.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/web/components/Header/LoginButton.css.ts
- apps/web/components/Header/LoginButton.tsx
🧰 Additional context used
📓 Path-based instructions (1)
libs/island-ui/core/src/lib/DropdownMenu/DropdownMenu.css.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/island-ui/core/src/lib/DropdownMenu/DropdownMenu.css.ts (2)
25-25: LGTM! Good use of theme spacing values
The padding implementation correctly utilizes theme spacing tokens, ensuring consistency across the design system.
5-5: Verify visual consistency across implementations
The switch from fixed width to minWidth and the addition of padding might affect existing implementations.
Also applies to: 25-25
✅ Verification successful
No visual impact expected from minWidth change
The change from fixed width to minWidth is safe as there are no implementations that depend on a specific fixed width. The only other minWidth usage found is in an unrelated touchable component. The change provides better flexibility while maintaining a minimum size constraint.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all DropdownMenu implementations to assess impact
rg -l "DropdownMenu" --type tsx
# Check for any custom width overrides that might conflict
ast-grep --pattern 'className={$_}' | rg -A 2 "width|min-width"
Length of output: 125
Script:
#!/bin/bash
# Search for all DropdownMenu implementations
rg -l "DropdownMenu"
# Check for any custom width overrides in files containing DropdownMenu
rg -B 2 -A 2 "DropdownMenu.*width"
# Look for style overrides specifically for DropdownMenu
ast-grep --pattern 'const $_ = {
$$$
width: $_,
$$$
}'
Length of output: 80286
Script:
#!/bin/bash
# Search for DropdownMenu style overrides in files that use it
rg -A 2 -B 2 "DropdownMenu.*style"
# Look for width-related props passed to DropdownMenu
rg -A 2 -B 2 "DropdownMenu.*width"
# Check for any custom styling that might affect width
ast-grep --pattern 'const $_ = {
$$$
minWidth: $_,
$$$
}'
Length of output: 799
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17185 +/- ##
==========================================
- Coverage 35.72% 35.72% -0.01%
==========================================
Files 6926 6926
Lines 147734 147724 -10
Branches 42092 42090 -2
==========================================
- Hits 52779 52770 -9
+ Misses 94955 94954 -1 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 32 Total Test Services: 0 Failed, 30 Passed Test ServicesThis report shows up to 10 services
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Set DropdownMenu styles to use minWidth
What
The className I passed down as props to the DropdownMenu isn't working on dev (works locally). There I was setting the width to be 193px. But since the component itself has a width style then it is what gets chosen.
Instead I've edited the DropdownMenu styles so we are now using "minWidth" instead of "width".
This way we are flexible when it comes to the menu item text
Screenshots / Gifs
Before
After
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes