-
Notifications
You must be signed in to change notification settings - Fork 2
refactor: migrate complex CSS calculations to clamp
#97
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
base: master
Are you sure you want to change the base?
Conversation
❌ Deploy Preview for curate-v2 failed. Why did it fail? →
|
WalkthroughThis PR replaces bespoke responsive calc() class strings and a local Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/ItemCard/index.tsx (1)
29-36: Inconsistent migration: mixing old and new approaches.The outer div still uses
cnandresponsiveSizefor inline styles (line 35), while the inner div (lines 38-41) usesclsxwith the new fluid classes. This creates an inconsistent styling approach within the same component.
♻️ Duplicate comments (2)
web/src/pages/SubmitItem/ItemField/FieldInput/TextInput.tsx (1)
4-4: Partial migration: responsiveSize still imported and used.The file migrates the width calculation to
lg:w-fluid-200-720but retainsresponsiveSizefor themarginBottominline style (line 12). This creates the same mixed approach seen in other files.web/src/pages/SubmitList/index.tsx (1)
35-43: Mixed migration approach: fluid classes and responsiveSize coexist.The container migrates
paddingBottomto the fluid classpb-fluid-76-96(line 38) but retainsresponsiveSizefor inlinepaddingInlineandpaddingTopstyles (lines 41-42). This creates the same partial migration pattern seen across multiple files.
🧹 Nitpick comments (2)
web/src/pages/SubmitItem/index.tsx (1)
39-47: Good migration step; consider finishing the move offresponsiveSizehere (paddingInline/paddingTop) for consistency.
Right now onlypaddingBottommoved topb-fluid-76-96, while inlineresponsiveSize(...)remains for the other paddings. If the intent is full “clamp utilities” adoption, it’d be cleaner to replace those too (and then drop theresponsiveSizeimport).web/src/pages/SubmitList/ItemParameters/Deposit.tsx (1)
8-10: PreferclassName={BASE_CONTAINER_STYLE}here (no need forcn()with a single static class).
This is equivalent and matches the other SubmitList pages that now useBASE_CONTAINER_STYLEdirectly.- <div className={cn(BASE_CONTAINER_STYLE)}> + <div className={BASE_CONTAINER_STYLE}>Also applies to: 18-20
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
web/src/components/InformationCards/RegistryInformationCard/TopInfo.tsx(2 hunks)web/src/components/ItemCard/index.tsx(2 hunks)web/src/components/RegistryCard/RegistryInfo.tsx(1 hunks)web/src/components/StatDisplay.tsx(1 hunks)web/src/global.css(1 hunks)web/src/layout/Header/navbar/DappList.tsx(1 hunks)web/src/layout/Header/navbar/Menu/Settings/index.tsx(3 hunks)web/src/pages/AllLists/index.tsx(1 hunks)web/src/pages/AttachmentDisplay/index.tsx(1 hunks)web/src/pages/Home/Stats/index.tsx(1 hunks)web/src/pages/Home/index.tsx(1 hunks)web/src/pages/Settings/index.tsx(1 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/AddressInput.tsx(1 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/FileInput.tsx(2 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/ImageInput.tsx(2 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/LinkInput.tsx(1 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/LongTextInput.tsx(1 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/NumberInput.tsx(1 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/TextInput.tsx(1 hunks)web/src/pages/SubmitItem/ItemField/FieldInput/constants.ts(0 hunks)web/src/pages/SubmitItem/Policy/Info.tsx(1 hunks)web/src/pages/SubmitItem/index.tsx(1 hunks)web/src/pages/SubmitList/DeployList/ListDetails.tsx(1 hunks)web/src/pages/SubmitList/ItemParameters/CustomName.tsx(1 hunks)web/src/pages/SubmitList/ItemParameters/Deposit.tsx(2 hunks)web/src/pages/SubmitList/ItemParameters/Fields/index.tsx(2 hunks)web/src/pages/SubmitList/ListParameters/Description.tsx(1 hunks)web/src/pages/SubmitList/ListParameters/LogoUpload.tsx(2 hunks)web/src/pages/SubmitList/ListParameters/Policy.tsx(2 hunks)web/src/pages/SubmitList/ListParameters/Title.tsx(2 hunks)web/src/pages/SubmitList/constants.ts(1 hunks)web/src/pages/SubmitList/index.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- web/src/pages/SubmitItem/ItemField/FieldInput/constants.ts
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: kyrers
Repo: kleros/curate-v2 PR: 89
File: web/src/styles/breakpoints.ts:1-1
Timestamp: 2025-11-14T22:12:39.974Z
Learning: In the kleros/curate-v2 repository, the kleros/ui-components-library overrides Tailwind's lg breakpoint to 900px (not the default 1024px). The LG_BREAKPOINT constant in web/src/styles/breakpoints.ts is set to 900 to match this override. This configuration is imported via the source directive in global.css.
📚 Learning: 2025-11-14T22:12:39.974Z
Learnt from: kyrers
Repo: kleros/curate-v2 PR: 89
File: web/src/styles/breakpoints.ts:1-1
Timestamp: 2025-11-14T22:12:39.974Z
Learning: In the kleros/curate-v2 repository, the kleros/ui-components-library overrides Tailwind's lg breakpoint to 900px (not the default 1024px). The LG_BREAKPOINT constant in web/src/styles/breakpoints.ts is set to 900 to match this override. This configuration is imported via the source directive in global.css.
Applied to files:
web/src/pages/AllLists/index.tsxweb/src/pages/SubmitList/constants.tsweb/src/components/ItemCard/index.tsxweb/src/layout/Header/navbar/DappList.tsxweb/src/global.css
📚 Learning: 2024-11-04T13:34:45.425Z
Learnt from: Harman-singh-waraich
Repo: kleros/curate-v2 PR: 65
File: web/src/utils/atlas/confirmEmail.ts:24-36
Timestamp: 2024-11-04T13:34:45.425Z
Learning: In the `confirmEmail` function in `web/src/utils/atlas/confirmEmail.ts`, input validation for the address is performed in the calling component, and additional checks are done in the API call itself. Input validation does not need to be repeated in this function.
Applied to files:
web/src/pages/SubmitItem/ItemField/FieldInput/AddressInput.tsx
📚 Learning: 2024-11-04T13:39:19.748Z
Learnt from: Harman-singh-waraich
Repo: kleros/curate-v2 PR: 65
File: web/src/utils/atlas/updateEmail.ts:21-38
Timestamp: 2024-11-04T13:39:19.748Z
Learning: For the `updateEmail` function in `web/src/utils/atlas/updateEmail.ts`, input validation and error handling are performed in the component that utilizes it. Therefore, additional checks within this utility function are unnecessary.
Applied to files:
web/src/pages/SubmitItem/ItemField/FieldInput/AddressInput.tsx
📚 Learning: 2024-11-04T13:33:39.755Z
Learnt from: Harman-singh-waraich
Repo: kleros/curate-v2 PR: 65
File: web/src/pages/Settings/index.tsx:18-26
Timestamp: 2024-11-04T13:33:39.755Z
Learning: In `web/src/pages/Settings/index.tsx`, the parent route handles the default `/settings` route, error boundaries, and loading states, so it's not necessary to include them in this component.
Applied to files:
web/src/pages/Settings/index.tsxweb/src/layout/Header/navbar/Menu/Settings/index.tsx
📚 Learning: 2024-11-04T13:31:54.080Z
Learnt from: Harman-singh-waraich
Repo: kleros/curate-v2 PR: 65
File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx:72-96
Timestamp: 2024-11-04T13:31:54.080Z
Learning: In the React TypeScript component `FormContactDetails` in `web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx`, error handling and user feedback are implemented within the `addUser` and `updateEmail` functions, which display toasts to the user.
Applied to files:
web/src/pages/Settings/index.tsxweb/src/pages/SubmitList/ListParameters/LogoUpload.tsxweb/src/layout/Header/navbar/Menu/Settings/index.tsx
📚 Learning: 2024-11-04T13:42:03.737Z
Learnt from: Harman-singh-waraich
Repo: kleros/curate-v2 PR: 65
File: web/src/components/ActionButton/Modal/EvidenceUpload.tsx:74-82
Timestamp: 2024-11-04T13:42:03.737Z
Learning: In `web/src/components/ActionButton/Modal/EvidenceUpload.tsx`, the `uploadFile` function already manages error handling and logging, so additional error handling after calling it is unnecessary.
Applied to files:
web/src/pages/SubmitItem/ItemField/FieldInput/FileInput.tsxweb/src/pages/SubmitList/ListParameters/LogoUpload.tsxweb/src/pages/SubmitItem/ItemField/FieldInput/ImageInput.tsx
📚 Learning: 2024-11-04T13:29:55.971Z
Learnt from: Harman-singh-waraich
Repo: kleros/curate-v2 PR: 65
File: web/src/context/AtlasProvider.tsx:176-193
Timestamp: 2024-11-04T13:29:55.971Z
Learning: In the `web/src/context/AtlasProvider.tsx` file of the React application, the `authoriseUser` function handles session invalidation and delegates error handling and user notifications to the functions that invoke it. Therefore, adding error notifications within `authoriseUser` is unnecessary.
Applied to files:
web/src/pages/SubmitList/ListParameters/LogoUpload.tsxweb/src/pages/SubmitList/ListParameters/Policy.tsx
🧬 Code graph analysis (6)
web/src/pages/SubmitList/ListParameters/Description.tsx (2)
web/src/context/SubmitListContext.tsx (1)
useSubmitListContext(123-123)web/src/pages/SubmitList/constants.ts (1)
BASE_CONTAINER_STYLE(1-1)
web/src/pages/SubmitList/ItemParameters/Fields/index.tsx (1)
web/src/pages/SubmitList/constants.ts (1)
BASE_CONTAINER_STYLE(1-1)
web/src/pages/SubmitList/ItemParameters/Deposit.tsx (2)
web/src/utils/index.ts (1)
cn(44-46)web/src/pages/SubmitList/constants.ts (1)
BASE_CONTAINER_STYLE(1-1)
web/src/pages/SubmitList/ListParameters/LogoUpload.tsx (1)
web/src/pages/SubmitList/constants.ts (1)
BASE_CONTAINER_STYLE(1-1)
web/src/pages/SubmitList/ItemParameters/CustomName.tsx (2)
web/src/context/SubmitListContext.tsx (1)
useSubmitListContext(123-123)web/src/pages/SubmitList/constants.ts (1)
BASE_CONTAINER_STYLE(1-1)
web/src/pages/SubmitList/ListParameters/Policy.tsx (1)
web/src/pages/SubmitList/constants.ts (1)
BASE_CONTAINER_STYLE(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - curate-v2
- GitHub Check: Header rules - curate-v2
- GitHub Check: Pages changed - curate-v2
🔇 Additional comments (31)
web/src/pages/Home/Stats/index.tsx (1)
71-76: Thepb-4!andlg:pb-0!syntax is correct for Tailwind CSS v4 (currently at ^4.1.16 in this project). In Tailwind v4, the important modifier uses a trailing!after the utility class, unlike v3 which used a leading!prefix. No changes needed.Likely an incorrect or invalid review comment.
web/src/pages/Settings/index.tsx (1)
4-12: LGTM; just double-check the combinedpx-4+lg:px-fluid-0-32doesn’t regress the desktop spacing you expect.web/src/pages/AttachmentDisplay/index.tsx (1)
9-22: All three padding utilities are properly defined in the project. The--spacing-fluid-24-136,--spacing-fluid-32-80, and--spacing-fluid-76-96CSS variables are defined inweb/src/global.cssas responsive fluid spacing utilities usingclamp(), and can be used as Tailwind classes as shown in the code.web/src/pages/SubmitItem/Policy/Info.tsx (1)
11-11: Thew-fluid-342-618width utility is correctly defined and ensures smooth responsive scaling. The AlertMessage will fluidly transition from84vwon mobile to a maximum of618pxwithout jarring jumps at the 900pxlgboundary.web/src/pages/SubmitList/DeployList/ListDetails.tsx (1)
14-19: Nice simplification;p-fluid-24-32exists and applies as expected (scales 24–32px from 375–1250px viewport width).web/src/components/StatDisplay.tsx (1)
15-15: No issues found. Thelg:mb-fluid-16-30utility class is properly defined via the Tailwind v4 configuration and will apply the intended responsive bottom margin spacing on desktop viewports.web/src/pages/SubmitList/constants.ts (1)
1-1: LGTM!The addition of
lg:w-fluid-442-700-900successfully replaces the complex landscape width calculation with a CSS clamp-based token, aligning perfectly with the PR objective.web/src/pages/SubmitList/ItemParameters/Fields/index.tsx (1)
9-9: LGTM!Successfully migrated from
cncomposition with landscape width calculations to direct usage ofBASE_CONTAINER_STYLE. This simplifies the className logic and aligns with the migration to fluid spacing tokens.Also applies to: 43-43
web/src/pages/SubmitList/ListParameters/Description.tsx (1)
6-6: LGTM!Clean migration from
cn-based composition to direct usage ofBASE_CONTAINER_STYLE, consistent with the broader refactoring effort.Also applies to: 14-14
web/src/pages/SubmitList/ListParameters/Policy.tsx (1)
12-12: LGTM!Consistent migration from
cncomposition to directBASE_CONTAINER_STYLEusage, aligning with the refactoring pattern across all SubmitList components.Also applies to: 37-37
web/src/pages/SubmitList/ListParameters/LogoUpload.tsx (1)
11-11: LGTM!Successfully migrated to direct
BASE_CONTAINER_STYLEusage, maintaining consistency with the other SubmitList components.Also applies to: 54-54
web/src/pages/SubmitItem/ItemField/FieldInput/FileInput.tsx (1)
8-8: LGTM!Excellent migration from
cntoclsxwith proper usage of fluid spacing tokens (w-fluid-200-720,mb-fluid-150-72). The className composition is cleaner and leverages the new CSS clamp-based utilities as intended.Note: The
mb-fluid-150-72token uses reverse scaling (150px at mobile → 72px at desktop), which is intentional for this use case.Also applies to: 30-36
web/src/global.css (1)
27-60: The Tailwind configuration is already correctly set up for Tailwind v4. The@sourcedirective in global.css (line 3) properly enables Tailwind to scan the kleros/ui-components-library, and the custom CSS variables defined in the@themeblock are being used correctly throughout the codebase via arbitrary values (e.g.,lg:p-[48px_var(--spacing-fluid-0-32)_60px]). No additional mapping or configuration is needed.web/src/pages/SubmitItem/ItemField/FieldInput/AddressInput.tsx (1)
24-28: Ensurelg:w-fluid-200-720is defined + consider migratingmarginBottomtoo.
The static width class is cleaner, but it depends on thew-fluid-200-720token being present in Tailwind/global CSS. Also notestyle={{ marginBottom: responsiveSize(...) }}is still a runtime calc path if the goal is “all clamp”.web/src/pages/SubmitItem/ItemField/FieldInput/LongTextInput.tsx (1)
8-12: Verify the new utility classes exist (lg:w-fluid-200-720,custom-scrollbar).
Change looks fine, but make sure the new width token and scrollbar class are defined and survive Tailwind’s content scanning.web/src/components/RegistryCard/RegistryInfo.tsx (1)
21-23: Confirm the CSS vars exist and the lg grid template renders correctly at the 900px breakpoint.
Nice direction (tokenized, no complex calc), but the class depends on--spacing-fluid-80-100-900/--spacing-fluid-100-150-900being defined and the arbitrarygrid-cols-[...]being picked up by Tailwind. (Also: using900matches the repo’slg=900pxoverride.)web/src/pages/SubmitList/ListParameters/Title.tsx (1)
6-7: LGTM: container now usesBASE_CONTAINER_STYLEdirectly.
Clean simplification and consistent with the removal of landscape calc constants.Also applies to: 15-16
web/src/pages/SubmitList/ItemParameters/CustomName.tsx (1)
5-6: LGTM:BASE_CONTAINER_STYLEapplied directly.
Simplifies layout styling and removes dependence on landscape calc constants.Also applies to: 11-12
web/src/pages/SubmitItem/ItemField/FieldInput/LinkInput.tsx (1)
23-28: Verifylg:w-fluid-200-720matches the old width behavior.
Change is fine structurally; just make sure the new token exists and doesn’t regress field widths on>= lg.web/src/pages/SubmitItem/ItemField/FieldInput/NumberInput.tsx (1)
11-16: Verifylg:w-fluid-200-720is defined and applied forNumberFieldtoo.
Assuming the token exists, this is a straightforward and consistent cleanup.web/src/components/InformationCards/RegistryInformationCard/TopInfo.tsx (2)
63-66: LGTM! Clean migration to clsx and fluid spacing.The className composition correctly migrates from
cntoclsxand introduces the new responsive gap utilities that align with the CSS clamp approach.
7-13: Mixed migration approach is intentional, but should be completed in this file.The file uses both
responsiveSize(lines 38, 40) and new fluid spacing classes (line 65), which is consistent with an ongoing codebase-wide migration from inline responsive calculations to CSS-based fluid spacing variables. While this partial migration appears intentional across the codebase (86 existing usages ofresponsiveSizeremain), completing the conversion in this file would improve consistency. Replace the inlineresponsiveSizecalls on lines 38 and 40 with corresponding fluid spacing classes to align with the new pattern established on line 65.web/src/pages/SubmitItem/ItemField/FieldInput/TextInput.tsx (1)
11-11: Clean migration to fluid width class.The responsive width correctly uses the new
lg:w-fluid-200-720class, which should use CSS clamp under the hood.web/src/components/ItemCard/index.tsx (1)
12-12: Well-structured CSS variable and fluid spacing usage.The grid column definition using
var(--spacing-fluid-150-180-900)and the gap classlg:gap-fluid-16-36-900correctly leverage the new clamp-based utilities.Also applies to: 38-41
web/src/pages/SubmitItem/ItemField/FieldInput/ImageInput.tsx (3)
6-8: Clean import migration.Correctly removes
cnand addsclsxfor the new className composition approach.
31-34: Excellent: Complete migration to fluid utilities.This component fully migrates to the new approach with:
- Fluid width classes:
w-[84vw] lg:w-fluid-200-720- Fluid margin:
mb-fluid-150-72- Custom text formatting classes
No leftover
responsiveSizeinline styles, making this a model for the complete migration.
37-37: Verify message prop concatenation behavior.The
messageprop now concatenatesfieldProp.descriptionwith the file uploader message using\n. Ensure this displays correctly in both mobile and desktop views, especially with the[&_small]:whitespace-pre-lineclass applied.web/src/layout/Header/navbar/Menu/Settings/index.tsx (2)
8-8: Clean addition of clsx.Correctly introduces
clsxfor className composition.
37-42: Consistent migration to clsx with distinct responsive sizing strategies.The component correctly uses
clsxfor both the container and Tabs. The code at lines 37-42 employs standard Tailwind utilities, while line 46 usesscaleutilities (w-scale-300-424-300,px-scale-8-32-300). This represents a deliberate choice:scaleutilities apply linear scaling viacalc(), whilefluidutilities (used elsewhere in the codebase) applyclamp()with hard min/max boundaries for more constrained responsive behavior.web/src/pages/SubmitList/index.tsx (2)
19-19: Clean clsx import.Correctly introduces
clsxfor the new className composition.
35-39: Good use of clsx and fluid utilities.The component correctly migrates to
clsxand introduces fluid spacing classes (pb-fluid-76-96,lg:px-fluid-25-65).Also applies to: 47-47
Resolves #96.
PR-Codex overview
This PR focuses on updating styles across various components in the application by replacing complex CSS calculations with simpler, more readable utility classes. It enhances responsiveness and maintains the visual consistency of the UI.
Detailed summary
BASE_CONTAINER_LANDSCAPE_WIDTH_CALCand updated to useBASE_CONTAINER_STYLE.clsxfor better readability.TextInput,LongTextInput, etc.).Summary by CodeRabbit
Refactor
Style
New Features
✏️ Tip: You can customize this high-level summary in your review settings.