-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
resources box fix #2512
resources box fix #2512
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a series of minor modifications across several components in the landing project. The changes primarily focus on styling adjustments, dependency updates, and subtle improvements to user interface components. The most notable updates include removing background classes, updating the Headless UI library version, and refining resource selection and bridging functionalities. The modifications aim to enhance the visual presentation and user interaction without significantly altering the core application logic. Changes
Suggested reviewers
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
CodeRabbit Configuration File (
|
You are out of MentatBot reviews. Your usage will refresh December 16 at 08:00 AM. |
Failed to generate code suggestions for PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
landing/src/components/modules/bridge-out-step-1.tsx (1)
218-224
: Consider extracting complex gradient stylesThe conditional styling logic is correct, but the gradient styles could be moved to a utility class for better maintainability and reuse.
Consider creating a utility class:
-className={ - "w-full " + - (!realmEntityId - ? "dark:bg-dark-brown dark:[background:linear-gradient(45deg,#1a1311,#1a1311)_padding-box,conic-gradient(from_var(--border-angle),#8b7355_80%,_#c6a366_86%,_#e5c088_90%,_#c6a366_94%,_#8b7355)_border-box] border border-transparent animate-border [background:linear-gradient(45deg,#ffffff,#ffffff)_padding-box,conic-gradient(from_var(--border-angle),#8b7355_80%,_#c6a366_86%,_#e5c088_90%,_#c6a366_94%,_#8b7355)_border-box]" - : "border-gold/40") -} +className={`w-full ${!realmEntityId ? "gradient-border-animate" : "border-gold/40"}`}landing/src/components/modules/bridge-in.tsx (1)
Line range hint
134-137
: Enhance error message specificityWhile the error handling is good, the error message could be more specific to help users understand and resolve the issue.
Consider updating the error message:
-throw new Error("No valid resources selected"); +throw new Error("Please select at least one resource and specify an amount greater than 0");landing/src/components/ui/elements/ListSelect.tsx (2)
Line range hint
69-94
: Consider accessibility improvementsThe ListboxOption implementation could benefit from:
- Adding
aria-label
for better screen reader support- Consider using
rem
instead of hardcoded pixel values for better accessibility (e.g.,h-3 w-3
)<ListboxOption key={option.id} + aria-label={`Select ${option.id}`} className={({ active }) => `overflow-visible relative cursor-pointer z-50 select-none py-2 flex items-center pl-8 text-gold ${ active ? "bg-gold/10 text-white/90" : "" }` } value={option.id} >
Line range hint
1-105
: Consider adding error boundarySince this component is crucial for resource selection, consider wrapping it in an error boundary to gracefully handle any rendering errors, especially given the recent Headless UI upgrade.
Would you like me to provide an example implementation of an error boundary component?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
landing/.env.production
(1 hunks)landing/package.json
(1 hunks)landing/src/components/modules/bridge-in.tsx
(2 hunks)landing/src/components/modules/bridge-out-step-1.tsx
(2 hunks)landing/src/components/modules/bridge-out-step-2.tsx
(1 hunks)landing/src/components/modules/swap-panel.tsx
(1 hunks)landing/src/components/modules/top-navigation-view.tsx
(2 hunks)landing/src/components/ui/SelectResources.tsx
(0 hunks)landing/src/components/ui/elements/ListSelect.tsx
(5 hunks)
💤 Files with no reviewable changes (1)
- landing/src/components/ui/SelectResources.tsx
✅ Files skipped from review due to trivial changes (5)
- landing/.env.production
- landing/src/components/modules/swap-panel.tsx
- landing/package.json
- landing/src/components/modules/bridge-out-step-2.tsx
- landing/src/components/modules/top-navigation-view.tsx
🔇 Additional comments (5)
landing/src/components/modules/bridge-out-step-1.tsx (1)
159-159
: LGTM: Improved container styling
The updated container styling with max-h-[calc(75vh-100px)]
and overflow-y-auto
ensures better content scrolling behavior.
landing/src/components/modules/bridge-in.tsx (1)
173-173
: LGTM: Consistent container styling
The container styling matches bridge-out-step-1.tsx, maintaining UI consistency across bridge components.
landing/src/components/ui/elements/ListSelect.tsx (3)
3-3
: Verify Headless UI version compatibility
The flat imports pattern (ListboxButton
, ListboxOption
, etc.) is specific to @headlessui/react v2. Let's ensure the package.json has the correct version.
✅ Verification successful
Headless UI version is correctly configured
The package.json specifies @headlessui/react version ^2.2.0, which is compatible with the flat imports pattern used in the component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check package.json for @headlessui/react version
cat landing/package.json | jq '.dependencies["@headlessui/react"]'
Length of output: 77
Line range hint 34-48
: Review z-index hierarchy and color opacity
A few concerns about the styling:
- The parent has
z-100
while the button hasz-0
, which might cause unexpected stacking behavior - Using opacity in background colors (
bg-gold/10
,bg-gold/20
) might cause unintended color blending with elements underneath
61-67
: Verify anchor prop documentation and scrollbar accessibility
The anchor="bottom start"
prop is new. A few suggestions:
- Add a comment documenting the valid anchor values and behavior
- The
no-scrollbar
class might hide scrollbars needed for accessibility
✅ Verification successful
Let me search for more context about the anchor prop and scrollbar-related styles.
Let me check the tailwind configuration and index.css for scrollbar-related styles.
Let me check the Headless UI documentation for the anchor prop.
The no-scrollbar
class is intentional and accessibility is maintained
The review comment's concerns can be addressed:
-
The
anchor="bottom start"
prop is a valid positioning prop in Headless UI v2 (@headlessui/react ^2.2.0 is used in the project). -
The
no-scrollbar
utility class is properly defined in the Tailwind configuration and only hides the scrollbar visually while maintaining scroll functionality. This is evident from the utility definition in the Tailwind config:
".no-scrollbar": {
"-ms-overflow-style": "none",
"scrollbar-width": "none",
"&::-webkit-scrollbar": {
display: "none"
}
}
The component remains accessible as:
- Keyboard navigation is unaffected
- The content is still scrollable via mouse wheel, touch, or keyboard
- The
overflow-scroll
class ensures scroll functionality is preserved
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for no-scrollbar usage and definition
rg "no-scrollbar" landing/src/
Length of output: 190
Script:
#!/bin/bash
# Check for ListboxOptions usage and definition
ast-grep --pattern 'ListboxOptions'
# Check for tailwind scrollbar related classes
rg -l "@tailwind|scrollbar" landing/src/
Length of output: 477
Script:
#!/bin/bash
# Check index.css content for scrollbar styles
cat landing/src/index.css
# Check for tailwind config
fd "tailwind.config" --exec cat {}
Length of output: 17257
Script:
#!/bin/bash
# Check for any other ListboxOptions usage with anchor prop
rg -A 2 "ListboxOptions.*anchor"
# Check package.json for @headlessui/react version
fd "package.json" --exec cat {}
Length of output: 12344
@@ -226,7 +226,7 @@ export const BridgeIn = () => { | |||
<div className="text-xs text-slate-500 uppercase mb-1">To Realm</div> | |||
|
|||
<Select onValueChange={(value) => setRealmEntityId(Number(value))}> | |||
<SelectTrigger className="w-full dark:[background:linear-gradient(45deg,#1a1311,#1a1311)_padding-box,conic-gradient(from_var(--border-angle),#8b7355_80%,_#c6a366_86%,_#e5c088_90%,_#c6a366_94%,_#8b7355)_border-box] light:[background:linear-gradient(45deg,#ffffff,#ffffff)_padding-box,conic-gradient(from_var(--border-angle),#b08c4f_80%,_#daa520_86%,_#ffd700_90%,_#daa520_94%,_#b08c4f)_border-box] border border-transparent animate-border"> | |||
<SelectTrigger className={"w-full " + (!realmEntityId ? "bg-dark-brown dark:[background:linear-gradient(45deg,#1a1311,#1a1311)_padding-box,conic-gradient(from_var(--border-angle),#8b7355_80%,_#c6a366_86%,_#e5c088_90%,_#c6a366_94%,_#8b7355)_border-box] border border-transparent animate-border [background:linear-gradient(45deg,#ffffff,#ffffff)_padding-box,conic-gradient(from_var(--border-angle),#8b7355_80%,_#c6a366_86%,_#e5c088_90%,_#c6a366_94%,_#8b7355)_border-box]" : "border-gold/15")}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consolidate duplicate gradient styles
The gradient styles are duplicated from bridge-out-step-1.tsx. This reinforces the need for a shared utility class.
Extract the styles to a shared utility class and update both components:
-className={"w-full " + (!realmEntityId ? "bg-dark-brown dark:[background:linear-gradient(45deg,#1a1311,#1a1311)_padding-box,conic-gradient(from_var(--border-angle),#8b7355_80%,_#c6a366_86%,_#e5c088_90%,_#c6a366_94%,_#8b7355)_border-box] border border-transparent animate-border [background:linear-gradient(45deg,#ffffff,#ffffff)_padding-box,conic-gradient(from_var(--border-angle),#8b7355_80%,_#c6a366_86%,_#e5c088_90%,_#c6a366_94%,_#8b7355)_border-box]" : "border-gold/15")}>
+className={`w-full ${!realmEntityId ? "gradient-border-animate" : "border-gold/15"}`}>
Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
SelectTrigger
component based on user selection.Bug Fixes
Style
Chores
@headlessui/react
to ensure compatibility with new features.