chore: tailwind and ui-component-library migration #130
Conversation
✅ Deploy Preview for kleros-escrow-v2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughRemoves styled-components and its theme augmentation, adds Tailwind tooling and global CSS, upgrades UI library, replaces the styled-components provider with a localStorage-backed ThemeProvider, migrates many components to className/clsx (cn) and updates Button/TextField APIs and several component interfaces. Changes
Sequence Diagram(s)(Skipped — changes are broad refactors and styling/tooling migrations rather than a new multi-component sequential flow.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (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). (6)
🔇 Additional comments (2)
Comment |
kyrers
force-pushed
the
chore/tailwind-and-ui-library-migraton
branch
from
f4d2ebb to
9641ce5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/src/context/ThemeProvider.tsx (1)
10-10: Consider validating the localStorage value.The code assumes the value from localStorage is a valid
Themetype, but malformed or corrupted data could cause issues. Consider adding validation:- const [theme, setTheme] = useLocalStorage<Theme>("theme", "dark"); + const [storedTheme, setTheme] = useLocalStorage<Theme>("theme", "dark"); + const theme: Theme = storedTheme === "light" || storedTheme === "dark" ? storedTheme : "dark";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
web/global.d.ts(1 hunks)web/package.json(2 hunks)web/src/app.tsx(3 hunks)web/src/context/StyledComponentsProvider.tsx(0 hunks)web/src/context/ThemeProvider.tsx(1 hunks)web/src/context/Web3Provider.tsx(2 hunks)web/src/global.css(1 hunks)web/src/hooks/useToggleThemeContext.tsx(1 hunks)web/src/layout/index.tsx(0 hunks)web/src/styles/commonStyles.ts(0 hunks)web/src/styles/customScrollbar.ts(0 hunks)web/src/styles/global-style.ts(0 hunks)web/src/styles/landscapeStyle.ts(0 hunks)web/src/styles/themes.ts(0 hunks)web/vite.config.js(2 hunks)
💤 Files with no reviewable changes (7)
- web/src/styles/global-style.ts
- web/src/styles/customScrollbar.ts
- web/src/layout/index.tsx
- web/src/styles/commonStyles.ts
- web/src/styles/landscapeStyle.ts
- web/src/context/StyledComponentsProvider.tsx
- web/src/styles/themes.ts
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/context/ThemeProvider.tsx (2)
web/src/hooks/useLocalStorage.ts (1)
useLocalStorage(3-22)web/src/hooks/useToggleThemeContext.tsx (1)
ToggleThemeProvider(10-16)
🪛 Biome (2.1.2)
web/src/global.css
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
⏰ 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). (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (11)
web/src/context/Web3Provider.tsx (1)
92-92: The hardcoded hex value"#4D00B4"in the AppKit configuration is correct and necessary. This is new code (not a modification), and AppKit's themeVariables requires CSS custom property strings. The color matches the brand primary purple used consistently throughout the codebase (SVGs, styled-components). No refactoring is needed.Likely an incorrect or invalid review comment.
web/global.d.ts (1)
12-12: LGTM!The addition of
export {}correctly converts this file to a module after removing the styled-components augmentation, which aligns with the migration to Tailwind CSS.web/src/app.tsx (2)
6-7: LGTM!The global CSS imports are correctly placed at the top level to ensure styles are applied application-wide.
11-11: LGTM!The replacement of StyledComponentsProvider with ThemeProvider correctly implements the new theming architecture. The provider wraps the entire application at the appropriate level.
Also applies to: 23-23, 46-46
web/src/context/ThemeProvider.tsx (1)
17-24: LGTM!The useEffect correctly applies the "dark" class to
document.documentElement, which is the standard approach for Tailwind CSS dark mode integration.web/src/global.css (3)
7-27: LGTM!The
@themeblock correctly defines custom CSS variables for colors, spacing, and other design tokens. The use ofcolor-mix()for opacity variants (line 21) is a modern approach that works well with Tailwind v4.
29-32: LGTM!The dark theme overrides for skeleton colors are correctly scoped to the
.darkclass, aligning with the ThemeProvider implementation.
105-132: LGTM!The custom scrollbar styles are well-implemented with proper transitions and theme-aware colors.
web/package.json (2)
72-72: Styled-components successfully removed from codebase.All styled-components dependencies have been completely removed from package.json and no remaining imports or usage patterns exist in the codebase.
72-72: No action required—@kleros/ui-components-library v3.6.0 is already integrated successfully.The codebase is actively using v3.6.0 across 30+ components (Button, Field, Card, Steps, Datepicker, FileUploader, Searchbar, AlertMessage, Tooltip, etc.) without reported breaking changes or compatibility issues. The CSS theme import and Tailwind CSS v4 integration are properly configured. The recent refactor to migrate from styled-components to Tailwind CSS indicates the upgrade was completed and tested.
web/vite.config.js (1)
5-5: The Tailwind CSS v4 Vite plugin integration is correct.The import and plugin registration are properly implemented. The project currently uses
@tailwindcss/viteat version^4.1.17as specified inweb/package.json. Regular dependency updates should be monitored to ensure compatibility with future releases.
There was a problem hiding this comment.
Actionable comments posted: 3
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/ConnectWallet/AccountDisplay.tsx (1)
51-51: Unsafe non-null assertion on potentially undefinedaddress.When the user is not connected and no
propAddressis provided,addresswill beundefined. The!assertion is misleading and could cause issues. Consider adding a guard or returning early.- return <label>{data ?? (isAddress(address!) ? shortenAddress(address) : address)}</label>; + if (!address) return null; + return <label>{data ?? (isAddress(address) ? shortenAddress(address) : address)}</label>;
♻️ Duplicate comments (1)
web/src/global.css (1)
1-5: Duplicate: Import order and redundant directive already flagged.This issue was already identified in a previous review. Please refer to the existing comment about moving
@import "tailwindcss"to the top and removing the redundant@tailwind utilitiesdirective.
🧹 Nitpick comments (8)
web/src/layout/Header/navbar/Debug.tsx (2)
6-9: Use<span>instead of<label>for semantic correctness.The
<label>element is semantically intended for form controls. Since this is purely presentational text without an associated input, use<span>instead.Apply this diff:
- <label className="text-[10px] leading-2.5 text-klerosUIComponentsStroke font-[Roboto_Mono,monospace]"> + <span className="text-[10px] leading-2.5 text-klerosUIComponentsStroke font-[Roboto_Mono,monospace]"> v{RELEASE_VERSION}{" "} <a className="text-inherit text-[10px] leading-2.5 font-[Roboto_Mono,monospace]"And update the closing tag at line 19:
- </label> + </span>
8-13: Remove redundant typography classes from the anchor.The anchor element already inherits
font-size,line-height, andfont-familyfrom its parent. Onlytext-inheritis needed to inherit the text color.Apply this diff:
<a - className="text-inherit text-[10px] leading-2.5 font-[Roboto_Mono,monospace]" + className="text-inherit" href={GIT_URL} target="_blank" rel="noreferrer"web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (1)
25-25: Nice refactor: validation as derived value.Converting
emailIsValidfrom state to a computed value eliminates unnecessary state management since it's directly derived fromemailInput.web/src/components/InfoCard.tsx (1)
1-23: Consider usingcnutility for consistency.This component uses
clsxdirectly (line 1), while other components in the PR use thecnutility fromsrc/utilswhich combinesclsxwithtwMerge. Thecnutility properly merges Tailwind classes and resolves conflicts.For consistency and to benefit from Tailwind class merging, consider:
-import clsx from "clsx"; import React from "react"; +import { cn } from "src/utils"; import InfoCircle from "svgs/icons/info-circle.svg"; interface IInfoCard { msg: string; className?: string; } const InfoCard: React.FC<IInfoCard> = ({ msg, className }) => { return ( <div - className={clsx( + className={cn( "grid grid-cols-[16px_auto] gap-fluid-6-8-300 items-center justify-start", "text-start text-klerosUIComponentsSecondaryText", className )} >web/src/layout/Header/navbar/Product.tsx (1)
1-41: Consider usingcnutility and simplify conditional className.Similar to
InfoCard.tsx, this component usesclsxdirectly instead of thecnutility. Additionally, line 29 uses a template literal for conditional rendering that could be simplified.-import clsx from "clsx"; +import { cn } from "src/utils"; const Product: React.FC<IProduct> = ({ text, url, Icon }) => { const [isImgLoaded, setIsImgLoaded] = useState(false); return ( <a href={url} target="_blank" rel="noopener noreferrer" - className={clsx( + className={cn( "flex flex-col items-center pt-4 pb-7 px-2 max-w-[100px] w-fluid-100-130 rounded-[3px] gap-2", "cursor-pointer bg-klerosUIComponentsLightBackground hover:bg-light-grey dark:hover:bg-klerosUIComponentsLightGrey", "hover:transition-[transform_0.15s,background-color_0.3s] hover:scale-[1.02]" )} > {typeof Icon === "string" ? ( <> {!isImgLoaded ? <Skeleton width={48} height={46} circle /> : null} <img - className={`w-12 h-12 ${isImgLoaded ? "block" : "hidden"}`} + className={cn("w-12 h-12", isImgLoaded ? "block" : "hidden")} alt={Icon} src={Icon} onLoad={() => setIsImgLoaded(true)} />web/src/layout/Header/navbar/Menu/Settings/General.tsx (1)
13-62: Consider usingcnutility and review arbitrary selector coupling.Two observations:
Inconsistent utility usage: This component imports
clsx(line 6) while other components use thecnutility fromsrc/utils. For consistency and proper Tailwind class merging, consider usingcn.Arbitrary selector coupling: Line 39 uses
[&_label]:cursor-pointerwhich assumesAddressOrNamecomponent renders a<label>element. This creates tight coupling between components. Consider verifying this component structure is stable or using a more explicit approach.-import clsx from "clsx"; +import { cn } from "src/utils"; const General: React.FC = () => { // ... rest of component return ( <div className="flex justify-center p-4"> <EnsureChain> <div className="flex flex-col justify-center mt-3"> {address && ( <div className="flex flex-col gap-3"> <div className="flex justify-center mt-3"> <IdenticonOrAvatar size="48" /> </div> <div - className={clsx( + className={cn( "flex justify-center", "[&>label]:text-base [&>label]:font-semibold [&>label]:text-klerosUIComponentsPrimaryText" )} > {/* ... */} <div - className={clsx( + className={cn( "flex h-[34px] gap-2 justify-center items-center",web/src/layout/Header/navbar/Menu/index.tsx (1)
47-54: Consider using a stable key instead of index.While using
indexas a key works here since thebuttonsarray is static, it's better practice to use a stable identifier liketextwhich is unique for each button.Apply this diff:
- {buttons.map(({ text, Icon, onPress }, index) => ( + {buttons.map(({ text, Icon, onPress }) => ( <div - key={index} + key={text} className={clsx("flex items-center min-h-8", "[&_.button-text]:block lg:[&_.button-text]:hidden")} >web/src/components/ConnectWallet/AccountDisplay.tsx (1)
26-29: Avoid unnecessary ENS avatar lookups when name is undefined.When
nameis undefined,normalize("")is passed touseEnsAvatar, triggering an API call that will never return a valid avatar. Add thequery.enabledoption to skip the query when there's no name.const { data: avatar } = useEnsAvatar({ name: normalize(name ?? ""), chainId: 1, + query: { + enabled: !!name, + }, });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (29)
web/package.json(2 hunks)web/src/app.tsx(3 hunks)web/src/components/ConnectWallet/AccountDisplay.tsx(3 hunks)web/src/components/ConnectWallet/index.tsx(2 hunks)web/src/components/EnsureAuth.tsx(1 hunks)web/src/components/InfoCard.tsx(1 hunks)web/src/components/LightButton.tsx(1 hunks)web/src/components/Overlay.tsx(1 hunks)web/src/components/OverlayPortal.tsx(1 hunks)web/src/global.css(1 hunks)web/src/layout/Header/DesktopHeader.tsx(1 hunks)web/src/layout/Header/Logo.tsx(1 hunks)web/src/layout/Header/MobileHeader.tsx(1 hunks)web/src/layout/Header/index.tsx(1 hunks)web/src/layout/Header/navbar/DappList.tsx(2 hunks)web/src/layout/Header/navbar/Debug.tsx(2 hunks)web/src/layout/Header/navbar/Explore.tsx(2 hunks)web/src/layout/Header/navbar/Menu/Help.tsx(2 hunks)web/src/layout/Header/navbar/Menu/Settings/General.tsx(2 hunks)web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx(2 hunks)web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx(2 hunks)web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx(3 hunks)web/src/layout/Header/navbar/Menu/Settings/Notifications/index.tsx(1 hunks)web/src/layout/Header/navbar/Menu/Settings/index.tsx(2 hunks)web/src/layout/Header/navbar/Menu/index.tsx(1 hunks)web/src/layout/Header/navbar/Product.tsx(2 hunks)web/src/layout/Header/navbar/index.tsx(2 hunks)web/src/layout/index.tsx(1 hunks)web/src/utils/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/package.json
- web/src/layout/index.tsx
- web/src/app.tsx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-28T14:07:11.735Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/layout/Header/navbar/index.tsx:56-56
Timestamp: 2024-11-28T14:07:11.735Z
Learning: In `web/src/layout/Header/navbar/index.tsx`, it's acceptable not to pass the optional `initialTab` prop when rendering the `Settings` component, as the component handles it with a fallback value.
Applied to files:
web/src/layout/Header/navbar/Menu/Settings/Notifications/index.tsxweb/src/layout/Header/navbar/Menu/Settings/General.tsxweb/src/layout/Header/navbar/Menu/index.tsxweb/src/layout/Header/navbar/Menu/Settings/index.tsxweb/src/layout/Header/navbar/Explore.tsxweb/src/layout/Header/navbar/Menu/Help.tsxweb/src/layout/Header/navbar/index.tsxweb/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx
📚 Learning: 2025-04-30T10:21:32.345Z
Learnt from: jaybuidl
Repo: kleros/escrow-v2 PR: 114
File: web/src/components/ScrollTop.tsx:1-33
Timestamp: 2025-04-30T10:21:32.345Z
Learning: In the ScrollTop component, the useEffect with an empty dependency array is intentional as the scrolling logic should only execute once when the component mounts. The component uses a hasScrolled ref to prevent re-executing the scroll logic.
Applied to files:
web/src/layout/Header/navbar/Menu/Settings/index.tsx
📚 Learning: 2024-11-28T13:58:33.048Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx:44-46
Timestamp: 2024-11-28T13:58:33.048Z
Learning: In `web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx`, when checking `user.emailUpdateableAt`, it's acceptable to use the non-null assertion operator `!` due to type assertion issues preventing standard checks.
Applied to files:
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx
📚 Learning: 2024-11-28T14:13:54.217Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/pages/Settings/EmailConfirmation/index.tsx:141-153
Timestamp: 2024-11-28T14:13:54.217Z
Learning: In the `EmailConfirmation` component, adding a cleanup function with an unsubscribed variable inside `useEffect` is unnecessary unless there's a specific need to handle component unmounting or prevent memory leaks.
Applied to files:
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx
🧬 Code graph analysis (11)
web/src/components/ConnectWallet/index.tsx (1)
web/src/consts/chains.ts (2)
SUPPORTED_CHAINS(9-11)DEFAULT_CHAIN(6-6)
web/src/layout/Header/navbar/Menu/Settings/Notifications/index.tsx (3)
web/src/layout/Header/navbar/index.tsx (1)
ISettings(24-27)web/src/components/EnsureChain.tsx (1)
EnsureChain(10-14)web/src/components/EnsureAuth.tsx (1)
EnsureAuth(17-45)
web/src/layout/Header/navbar/Menu/Settings/General.tsx (2)
web/src/components/EnsureChain.tsx (1)
EnsureChain(10-14)web/src/components/ConnectWallet/AccountDisplay.tsx (3)
IdenticonOrAvatar(18-36)AddressOrName(42-52)ChainDisplay(54-58)
web/src/components/Overlay.tsx (1)
web/src/utils/index.ts (1)
cn(41-43)
web/src/layout/Header/navbar/Menu/index.tsx (2)
web/src/layout/Header/navbar/index.tsx (2)
ISettings(24-27)IHelp(29-31)web/src/hooks/useToggleThemeContext.tsx (1)
useTheme(18-20)
web/src/layout/Header/navbar/Explore.tsx (1)
web/src/utils/index.ts (1)
cn(41-43)
web/src/components/LightButton.tsx (1)
web/src/utils/index.ts (1)
cn(41-43)
web/src/layout/Header/index.tsx (1)
web/src/utils/getGraphqlUrl.ts (1)
getGraphqlUrl(4-12)
web/src/layout/Header/navbar/index.tsx (3)
web/src/utils/index.ts (1)
cn(41-43)web/src/components/Overlay.tsx (1)
Overlay(9-11)web/src/layout/Header/navbar/Menu/Settings/General.tsx (1)
DisconnectWalletButton(8-11)
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx (1)
web/src/utils/wrapWithToast.ts (3)
infoToast(15-15)successToast(16-16)errorToast(17-17)
web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (2)
web/src/consts/index.ts (1)
EMAIL_REGEX(16-17)web/src/utils/date.ts (1)
timeLeftUntil(32-58)
🪛 Biome (2.1.2)
web/src/global.css
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
⏰ 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). (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (23)
web/src/layout/Header/navbar/Debug.tsx (2)
17-18: LGTM! Enhanced version display with Git metadata.The conditional rendering for
GIT_TAGSandGIT_DIRTYfollows the same pattern as the existingGIT_BRANCHcheck and provides useful debug information.
46-49: LGTM! Clean migration from styled-components.Replacing the styled
Containerwith a plaindivaligns with the Tailwind migration objectives. The wrapper correctly contains the child components without requiring additional styling.web/src/components/EnsureAuth.tsx (1)
34-43: LGTM!The refactoring from styled-components to plain HTML with Tailwind classes is clean. The Button prop updates (
onClick→onPress,disabled→isDisabled) correctly align with@kleros/ui-components-libraryv3.web/src/layout/Header/Logo.tsx (1)
6-11: LGTM!The Tailwind v4 syntax is correct: the
hover:[&_path]:fill-white/75uses arbitrary selectors to target SVG paths, and the/75opacity syntax aligns with v4's updated approach.web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/FormContact.tsx (1)
1-40: LGTM!The refactoring simplifies the component by moving validation logic to the parent. The
TextFieldintegration is clean, and the nested selector syntax ([&_input]:text-sm [&_label]:self-start) is valid Tailwind v4.web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/EmailVerificationInfo.tsx (2)
17-31: LGTM!Removing the unused event parameter from
resendVerificationEmailsimplifies the callback. The refactoring correctly maintains the async flow with toast notifications.
34-58: LGTM!The conversion from styled-components to Tailwind classes is well-executed. The Button styling with
[&_.button-text]selectors correctly targets internal elements using Tailwind v4's arbitrary selector syntax.web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx (1)
73-112: LGTM!The form refactoring from styled-components to plain HTML with Tailwind classes is clean. The custom utility
px-fluid-12-32-300should work with the definitions inglobal.css, and the button disabled logic is correctly preserved.web/src/layout/Header/navbar/DappList.tsx (1)
88-111: Fluid spacing utilities are properly configured.The @theme directive is correctly being used to define theme variables that instruct Tailwind to create new utility classes. The global.css file defines fluid spacing variables following the
--spacing-fluid-*namespace pattern (e.g.,--spacing-fluid-300-480,--spacing-fluid-8-24), which properly generates corresponding utility classes likew-fluid-300-480andpx-fluid-8-24. Each variable uses the CSSclamp()function for responsive fluid scaling across different viewport sizes, which is the correct approach for Tailwind v4's CSS-native design system.web/src/utils/index.ts (1)
40-43: LGTM! Standard Tailwind className composition utility.The
cnutility correctly combinesclsxfor conditional class composition withtwMergefor resolving Tailwind class conflicts. This is the recommended pattern for Tailwind CSS projects.web/src/components/ConnectWallet/index.tsx (2)
8-30: LGTM! Props updated for UI library v3.The
SwitchChainButtoncorrectly usesisDisabled(line 25) andonPress(line 27) to align with the@kleros/ui-components-libraryv3 API.
32-44: LGTM! Props updated consistently.The
ConnectButtoncorrectly usesisDisabled(line 38) andonPress(line 41), maintaining consistency with the UI library v3 API changes.web/src/layout/Header/navbar/Explore.tsx (1)
15-42: LGTM! Clean migration to utility-based styling.The component correctly:
- Uses the
cnutility for className composition- Implements active state logic based on pathname matching
- Applies responsive styles with
lg:breakpoint- Conditionally styles for mobile/desktop variants
The
isActivehelper (lines 18-19) properly handles both root path and nested routes.web/src/layout/Header/navbar/Menu/Settings/General.tsx (1)
8-11: LGTM! Button updated with onPress handler.The
DisconnectWalletButtoncorrectly usesonPress(line 10) to align with the UI library v3 API.web/src/layout/Header/navbar/Menu/Settings/Notifications/index.tsx (1)
8-31: LGTM! Clean migration to Tailwind utilities.The component has been cleanly migrated from styled-components to Tailwind utility classes. The layout structure and functionality are preserved, with straightforward className usage.
web/src/components/LightButton.tsx (1)
5-12: LGTM! Prop interface updated for UI library v3.The rename from
onClicktoonPressaligns with the@kleros/ui-components-libraryv3 API. All call sites ofLightButtonhave been successfully updated to useonPress.web/src/layout/Header/MobileHeader.tsx (1)
27-33: LGTM! Clean migration to utility classes.The refactor successfully replaces styled-components with Tailwind utilities. The arbitrary variant syntax
[&_.button-svg]:mr-0is correctly used to target nested SVG elements, and the switch fromonClicktoonPressaligns with the LightButton API update.web/src/components/Overlay.tsx (1)
1-11: LGTM! Clean component migration.The refactor from styled-components to a functional component with utility classes is well-executed. The
cnutility properly merges default classes with the optionalclassNameprop, providing good API flexibility.web/src/layout/Header/navbar/Menu/Help.tsx (1)
49-84: LGTM! Secure and accessible refactor.The migration properly adds
rel="noopener noreferrer"for external links, which prevents security vulnerabilities. The responsive layout with Tailwind utilities and hover effects using thegrouppattern are well-implemented.web/src/layout/Header/index.tsx (1)
10-33: LGTM! Proper Tailwind v4 syntax and responsive design.The migration correctly uses Tailwind v4 syntax with the important modifier at the end (
sticky!) and appropriate responsive utilities. The CSS variables passed to StatusBanner's theme prop are correctly formatted as regular CSS custom properties.web/src/layout/Header/navbar/index.tsx (1)
47-80: LGTM! Well-structured mobile navigation refactor.The migration properly uses the
cnutility for conditional class composition and maintains the overlay behavior with appropriate transitions. The switch fromonClicktoonPressis consistent with the LightButton API changes throughout the codebase.web/src/layout/Header/navbar/Menu/Settings/index.tsx (1)
27-27: No changes needed. The Settings component correctly implements the ISettings interface, which definesinitialTabas an optional prop. While the component doesn't destructureinitialTab, this is valid TypeScript—optional props don't require destructuring. DesktopHeader can pass it without error, and the component gracefully handles navigation via URL hash (location.hash.includes("#notifications")), providing the fallback behavior for tab selection. No breaking change exists.web/src/layout/Header/DesktopHeader.tsx (1)
69-71: Verify extra props passed to modal components.The code passes boolean state props (
isDappListOpen,isHelpOpen,isSettingsOpen) to the modal components, but these props are not declared in the interface definitions (IDappList,IHelp,ISettings). This could indicate either:
- The components were updated to use these props but interfaces weren't updated
- These props are unnecessary and will be ignored
Additionally, as noted in Settings/index.tsx, the
initialTabprop is being passed here but the Settings component no longer accepts it in its signature.Verify the component signatures and prop usage:
#!/bin/bash # Check DappList component signature echo "=== DappList component ===" rg -nP 'const DappList.*FC' -A 2 web/src/layout/Header/navbar/DappList # Check Help component signature echo "=== Help component ===" rg -nP 'const Help.*FC' -A 2 web/src/layout/Header/navbar/Menu/Help.tsx # Check Settings component signature echo "=== Settings component ===" rg -nP 'const Settings.*FC' -A 2 web/src/layout/Header/navbar/Menu/Settings/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
web/src/layout/Footer/index.tsx (1)
17-17: Consider cleaner margin control.The
[&_.button-svg]:mr-0arbitrary selector overrides default margins from the LightButton component. If this pattern appears frequently, consider adding a margin control prop to LightButton instead of using arbitrary selectors.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/ExternalLink.tsx(0 hunks)web/src/layout/Footer/index.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- web/src/components/ExternalLink.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/layout/Footer/index.tsx (1)
web/src/consts/socialmedia.tsx (1)
socialmedia(8-33)
⏰ 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). (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (2)
web/src/layout/Footer/index.tsx (2)
8-8: LGTM!The clsx import is correctly used for composing conditional class names in the Footer component.
28-32: Verify custom Tailwind colors are properly exposed by the library.The Footer component uses custom color classes
bg-klerosUIComponentsPrimaryPurpleanddark:bg-klerosUIComponentsLightBluethat are sourced from the imported@kleros/ui-components-library. Confirm that the library'stheme.cssproperly defines these as Tailwind--color-*theme variables (e.g.,--color-klerosUIComponentsPrimaryPurple) rather than as regular CSS variables, to ensure the utility classes are generated correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/src/global.css (1)
1-5: [Duplicate] Fix @import order and remove redundant directive.This issue has already been identified in a previous review. The
@import "tailwindcss"at line 4 violates CSS specification because it appears after other at-rules, and having both@tailwind utilitiesand@import "tailwindcss"is redundant in Tailwind v4.
🧹 Nitpick comments (1)
web/src/pages/NewTransaction/Timeline.tsx (1)
24-27: Consider using exact matching instead ofincludesfor route comparison.The current route matching logic using
includes()could unintentionally match overlapping paths if new routes are added later. While the current route set works correctly, a more robust approach would use exact matching orstartsWith().Apply this diff for a safer implementation:
- const currentItemIndex = Object.entries(routeToIndexMap).reduce( - (acc, [route, index]) => (location.pathname.includes(route) ? index : acc), - 0 - ); + const currentItemIndex = routeToIndexMap[location.pathname] ?? 0;Or if you need partial matching:
- const currentItemIndex = Object.entries(routeToIndexMap).reduce( - (acc, [route, index]) => (location.pathname.includes(route) ? index : acc), - 0 - ); + const matchedRoute = Object.keys(routeToIndexMap).find(route => + location.pathname === route || location.pathname.startsWith(route + '/') + ); + const currentItemIndex = matchedRoute ? routeToIndexMap[matchedRoute] : 0;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/src/global.css(1 hunks)web/src/pages/NewTransaction/HeroImage.tsx(1 hunks)web/src/pages/NewTransaction/Timeline.tsx(1 hunks)web/src/pages/NewTransaction/index.tsx(2 hunks)web/src/styles/breakpoints.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-28T13:58:33.048Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx:44-46
Timestamp: 2024-11-28T13:58:33.048Z
Learning: In `web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx`, when checking `user.emailUpdateableAt`, it's acceptable to use the non-null assertion operator `!` due to type assertion issues preventing standard checks.
Applied to files:
web/src/pages/NewTransaction/index.tsx
🧬 Code graph analysis (2)
web/src/pages/NewTransaction/index.tsx (3)
web/src/styles/breakpoints.ts (1)
LG_BREAKPOINT(1-1)web/src/consts/chains.ts (1)
DEFAULT_CHAIN(6-6)web/src/components/EnsureAuth.tsx (1)
EnsureAuth(17-45)
web/src/pages/NewTransaction/HeroImage.tsx (2)
web/src/hooks/useToggleThemeContext.tsx (1)
useTheme(18-20)web/src/styles/breakpoints.ts (1)
LG_BREAKPOINT(1-1)
🪛 Biome (2.1.2)
web/src/global.css
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
⏰ 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 - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (3)
web/src/pages/NewTransaction/HeroImage.tsx (1)
2-14: LGTM! Clean refactor to new theming approach.The migration from styled-components theming to the custom theme hook is well-executed. The array destructuring pattern and simplified theme comparison make the code cleaner while maintaining the same functionality.
web/src/styles/breakpoints.ts (1)
1-1: This is a custom viewport breakpoint constant for layout comparisons, not a Tailwind configuration. The value of 900px is appropriate for this project's specific design needs and does not require alignment with Tailwind defaults, as no Tailwind configuration exists in the repository.web/src/pages/NewTransaction/Timeline.tsx (1)
29-29: The Steps component usage requires verification against @kleros/ui-components-library v3.6.0 API documentation. Confirm that the component accepts theitemsarray andcurrentItemIndexprops with the current structure, as these prop names and their expected types are not verified in available documentation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
web/src/global.css (1)
1-5: Fix @import order and remove redundant directive.This issue was previously identified and remains unresolved. The import structure has two critical problems:
CSS spec violation:
@import "tailwindcss"at line 4 violates the CSS specification because all@importrules must precede other at-rules (except@charsetand@layer). Line 2's@tailwind utilitiesis an at-rule that appears before the import.Redundant directive: Per Tailwind CSS v4 documentation,
@import "tailwindcss"replaces@tailwind base,@tailwind components, and@tailwind utilities. Having both is redundant and may cause conflicts.🔎 Apply this fix:
-@import "../../node_modules/@kleros/ui-components-library/dist/assets/theme.css"; -@tailwind utilities; -@source "../../node_modules/@kleros/ui-components-library"; -@import "tailwindcss"; -@custom-variant dark (&:where(.dark, .dark *)); +@import "tailwindcss"; +@import "../../node_modules/@kleros/ui-components-library/dist/assets/theme.css"; +@source "../../node_modules/@kleros/ui-components-library"; +@custom-variant dark (&:where(.dark, .dark *));Based on Tailwind CSS v4 documentation and CSS specification.
🧹 Nitpick comments (1)
web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/CryptoSwap.tsx (1)
17-28: Consider aligning styling with GeneralEscrow for consistency.The CryptoSwap component has minor styling differences compared to GeneralEscrow:
- Line 17:
justify-centeris present here but absent in GeneralEscrow- Line 27:
px-2padding is present here but absent in GeneralEscrowSince both components serve the same purpose as escrow type options, they should have consistent styling.
🔎 Suggested alignment:
Option 1: Remove extra styles from CryptoSwap to match GeneralEscrow:
- <div className="flex flex-col gap-6 justify-center"> + <div className="flex flex-col gap-6"> <Card className={cn( "flex h-24 w-24 items-center justify-center cursor-pointer rounded-[20px]!", selected && "border border-klerosUIComponentsPrimaryBlue" )} onClick={handleSelect} > <Logo className="fill-klerosUIComponentsSecondaryPurple" /> </Card> - <p className="flex flex-wrap w-24 m-0 text-center px-2">Crypto Swap</p> + <p className="flex flex-wrap w-24 m-0 text-center">Crypto Swap</p> </div>Option 2: Add the same styles to GeneralEscrow if the extra spacing is intentional.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
web/src/components/StyledIcons/ClosedCircleIcon.tsx(0 hunks)web/src/global.css(1 hunks)web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/CryptoSwap.tsx(1 hunks)web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/GeneralEscrow.tsx(1 hunks)web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/index.tsx(1 hunks)web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/Info.tsx(1 hunks)web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/index.tsx(1 hunks)web/src/pages/NewTransaction/Header.tsx(1 hunks)web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx(5 hunks)web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx(1 hunks)web/src/pages/NewTransaction/NavigationButtons/PreviousButton.tsx(1 hunks)web/src/pages/NewTransaction/NavigationButtons/index.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- web/src/components/StyledIcons/ClosedCircleIcon.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-28T13:58:33.048Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx:44-46
Timestamp: 2024-11-28T13:58:33.048Z
Learning: In `web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx`, when checking `user.emailUpdateableAt`, it's acceptable to use the non-null assertion operator `!` due to type assertion issues preventing standard checks.
Applied to files:
web/src/pages/NewTransaction/NavigationButtons/NextButton.tsxweb/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsxweb/src/pages/NewTransaction/NavigationButtons/index.tsxweb/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/index.tsxweb/src/pages/NewTransaction/NavigationButtons/PreviousButton.tsx
📚 Learning: 2024-10-08T16:23:56.291Z
Learnt from: kemuru
Repo: kleros/escrow-v2 PR: 60
File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:180-180
Timestamp: 2024-10-08T16:23:56.291Z
Learning: The `refetchAllowance` function call is necessary in the `DepositPaymentButton` component to ensure the frontend updates correctly after an approval action.
Applied to files:
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx
📚 Learning: 2024-06-11T17:14:13.327Z
Learnt from: kemuru
Repo: kleros/escrow-v2 PR: 60
File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:63-64
Timestamp: 2024-06-11T17:14:13.327Z
Learning: Errors in the `useContractRead` hook for ERC20 token allowance checks in the `DepositPaymentButton` component are managed by the `wrapWithToast()` function, which logs errors in a notification popup. Additionally, the quantity for the approval is pre-validated and stored in the context of the NewTransactionContext.
Applied to files:
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx
🧬 Code graph analysis (4)
web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/GeneralEscrow.tsx (2)
web/src/context/NewTransactionContext.tsx (1)
useNewTransactionContext(56-60)web/src/utils/index.ts (1)
cn(41-43)
web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/CryptoSwap.tsx (2)
web/src/context/NewTransactionContext.tsx (1)
useNewTransactionContext(56-60)web/src/utils/index.ts (1)
cn(41-43)
web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (3)
web/src/context/NewTransactionContext.tsx (1)
useNewTransactionContext(56-60)web/src/utils/bufferRules.ts (1)
pickBufferFor(8-12)web/src/utils/validateAddress.ts (1)
ethAddressPattern(1-1)
web/src/pages/NewTransaction/NavigationButtons/PreviousButton.tsx (1)
web/src/utils/index.ts (1)
isEmpty(10-10)
🪛 Biome (2.1.2)
web/src/global.css
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
⏰ 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 - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (10)
web/src/pages/NewTransaction/Header.tsx (1)
8-8: LGTM! Clean migration to Tailwind v4.The CSS variable syntax
text-(length:--spacing-fluid-20-24)correctly uses Tailwind v4's type-hint syntax to disambiguate the variable's intended use as a font-size value. The responsive utilities and layout classes are well-structured.web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/index.tsx (1)
9-15: LGTM!Clean migration from styled-components to Tailwind utility classes. The flexbox layout is appropriate and the structure is maintained.
web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/GeneralEscrow.tsx (1)
17-28: LGTM!Well-executed migration to Tailwind v4 and the UI components library. The conditional border styling using
cnutility is clean, and the Tailwind v4 important modifier syntax (rounded-[20px]!) is correct.web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/Info.tsx (1)
9-23: LGTM!Clean refactor removing the styled wrapper. The responsive width utility (
lg:w-fluid-342-618) correctly references the custom fluid spacing token defined in global.css.web/src/pages/NewTransaction/EscrowDetails/TypeOfEscrow/EscrowOptions/index.tsx (1)
7-10: LGTM!Straightforward migration from styled-components to Tailwind utility classes. The flexbox layout with centered content and spacing is appropriate for displaying escrow options.
web/src/pages/NewTransaction/NavigationButtons/PreviousButton.tsx (1)
13-18: LGTM! Clean migration to utility classes.The conditional visibility pattern using
className={isEmpty(prevRoute) ? "hidden" : "flex"}is appropriate. The Button component correctly uses the new API (onPressinstead ofonClick) from @kleros/ui-components-library v3.web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx (2)
46-52: LGTM! Formatting improvements.The useMemo expressions are reformatted for better readability with no functional changes.
194-206: LGTM! Button API correctly updated.The Button component now uses
isDisabledandonPressprops, correctly aligning with @kleros/ui-components-library v3 API changes.web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx (1)
141-146: LGTM! Button API correctly updated.The Button component props have been correctly migrated to the v3 API:
disabled→isDisabledandonClick→onPress.web/src/pages/NewTransaction/NavigationButtons/index.tsx (1)
13-20: LGTM! Clean migration to Tailwind utility classes.The navigation buttons container uses appropriate Tailwind classes for a responsive, centered button layout. The conditional rendering logic correctly shows either the DepositPaymentButton or NextButton based on the current route.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/src/global.css (1)
1-5: CSS import order and redundancy issues already flagged.The critical issues with the import structure (CSS spec violation for
@importorder and redundant@tailwind utilitiesdirective) have already been identified in the previous review comment with a correct fix provided.
🧹 Nitpick comments (1)
web/src/pages/NewTransaction/EscrowDetails/Title/index.tsx (1)
10-12: Optional: Consider simplifying by passingsetEscrowTitledirectly.The
handleWritewrapper doesn't add any transformation or validation, so you could passsetEscrowTitledirectly to the TextField'sonChangeprop.🔎 Apply this diff to simplify:
- const handleWrite = (value: string) => { - setEscrowTitle(value); - }; - return ( <div className="flex flex-col items-center"> <Header text="Title" /> <TextField className="w-[84vw] lg:w-fluid-342-500" value={escrowTitle} - onChange={handleWrite} + onChange={setEscrowTitle} placeholder="e.g. Escrow with John" />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/global.css(1 hunks)web/src/pages/NewTransaction/EscrowDetails/Title/TextField.tsx(0 hunks)web/src/pages/NewTransaction/EscrowDetails/Title/index.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- web/src/pages/NewTransaction/EscrowDetails/Title/TextField.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-28T13:58:33.048Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx:44-46
Timestamp: 2024-11-28T13:58:33.048Z
Learning: In `web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx`, when checking `user.emailUpdateableAt`, it's acceptable to use the non-null assertion operator `!` due to type assertion issues preventing standard checks.
Applied to files:
web/src/pages/NewTransaction/EscrowDetails/Title/index.tsx
🧬 Code graph analysis (1)
web/src/pages/NewTransaction/EscrowDetails/Title/index.tsx (1)
web/src/context/NewTransactionContext.tsx (1)
useNewTransactionContext(56-60)
🪛 Biome (2.1.2)
web/src/global.css
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
⏰ 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 - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (2)
web/src/global.css (2)
7-150: Well-structured global styles and theme variables.The implementation of CSS variables, responsive spacing utilities, dark mode overrides, typography, and custom scrollbar styling is solid:
- Comprehensive color and spacing token system with fluid responsive utilities using
clamp()- Proper dark mode variable overrides scoped to
.darkclass- Cross-browser scrollbar styling with webkit pseudo-elements and
scrollbar-widthfallback- Standard CSS reset pattern and consistent typography referencing UI library theme variables
Once the import order issue (lines 1-5) is resolved, this provides a clean foundation for the Tailwind migration.
65-69: Ensure Open Sans font is properly loaded.The body style references "Open Sans", but there's no direct
@font-facedeclaration or font import in this file. The global.css imports theme.css from@kleros/ui-components-library(which presumably provides font loading), but this library is not listed in package.json dependencies or present in node_modules. Verify that the library is properly declared as a dependency or add explicit font loading (e.g., via Google Fonts link or @font-face) to ensure consistent typography instead of relying on the generic sans-serif fallback.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @web/src/components/Loader.tsx:
- Around line 17-19: The loader div in Loader.tsx should expose loading state to
assistive tech; add role="status" and a descriptive aria-label (e.g., "Loading")
on the container that renders KlerosIcon so screen readers announce the state,
and mark the decorative KlerosIcon as aria-hidden="true" to avoid redundant
announcements; update the JSX around the div that uses className, width, height
and KlerosIcon accordingly.
- Line 17: The falsy checks in the Loader component treat 0 as "not provided",
causing width={0} or height={0} to incorrectly get w-full/h-full classes; update
the conditional class logic in Loader.tsx to use nullish checks (e.g., check
width == null and height == null / or use width === undefined || width === null)
so only null/undefined add "w-full" or "h-full" while allowing 0 to pass
through, leaving the style={{ width, height }} unchanged.
- Line 18: The component uses an undefined CSS animation class
"animate-breathing" on KlerosIcon; add a breathing animation definition
(keyframes for e.g. scale/opacity pulsing) and a utility class named
"animate-breathing" in the global stylesheet or Tailwind config (e.g., define
@keyframes breathing and .animate-breathing { animation: breathing 2s infinite
ease-in-out; }) and ensure the global CSS (where @kleros/ui-components-library
is imported) is included so the class is available when KlerosIcon is rendered.
In @web/src/global.css:
- Around line 1-4: Move the @import "tailwindcss"; rule to the top of global.css
so it appears before any other @import or at-rule; ensure the file begins with
@import "tailwindcss"; and then retain the other imports like @import
"../../node_modules/@kleros/ui-components-library/dist/assets/theme.css",
@tailwind utilities, and @source in their current order to comply with CSS
@import ordering rules.
In @web/src/pages/AttachmentDisplay/Header.tsx:
- Line 17: The h1 in the Header component is using invalid Tailwind syntax
"text-(length:--spacing-fluid-16-24)"; replace that class on the h1 with a valid
Tailwind font-size utility or a bracketed CSS var reference such as using
text-[var(--spacing-fluid-16-24)] (or a conventional size like text-lg/text-xl),
or alternatively define a custom text size in the Tailwind theme and use that
token; update the className on the h1 element accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/src/components/FileViewer/Viewers/MarkdownViewer.tsxweb/src/components/FileViewer/index.tsxweb/src/components/Loader.tsxweb/src/global.cssweb/src/pages/AttachmentDisplay/Header.tsxweb/src/pages/AttachmentDisplay/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
web/src/components/Loader.tsx (1)
web/src/utils/index.ts (1)
cn(41-43)
🪛 Biome (2.1.2)
web/src/global.css
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
⏰ 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). (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (6)
web/src/components/FileViewer/Viewers/MarkdownViewer.tsx (1)
15-17: LGTM! Clean Tailwind migration.The styled-components Container has been successfully replaced with a Tailwind utility class. The
p-4class provides consistent 1rem padding on all sides.web/src/components/FileViewer/index.tsx (1)
16-33: LGTM! Correct Tailwind v4 syntax.The migration properly uses:
- Tailwind v4's end-positioned important modifier (
bg-klerosUIComponentsWhiteBackground!)- Custom CSS classes defined in
global.css(custom-scrollbar,shadow-custom)- Arbitrary value syntax for the border radius (
rounded-[3px])web/src/global.css (1)
7-165: Well-structured Tailwind v4 theme configuration.The
@themedirective and custom utilities are properly implemented:
- Comprehensive design token system with responsive spacing via
clamp()- Dark mode support with
.darkclass overrides- Custom scrollbar styling with proper hover/active states
- Integration with Kleros UI components library variables
web/src/pages/AttachmentDisplay/Header.tsx (1)
19-31: Complex but correct Button styling.The nested selector syntax using
[&_.button-text]and[&_.button-svg_path]correctly targets child elements within the Button component for hover and focus states. The use ofonPressaligns with the @kleros/ui-components-library v3 API.web/src/pages/AttachmentDisplay/index.tsx (1)
20-50: Excellent Tailwind migration.This refactor successfully replaces styled-components with Tailwind utilities:
- Responsive layout with mobile-first approach using
lg:breakpoint- Proper external link security with
rel="noopener noreferrer"- Custom fluid spacing tokens for responsive padding
- Clean component composition using
clsxfor conditional classesweb/src/components/Loader.tsx (1)
1-4: LGTM: Imports updated correctly for Tailwind migration.The imports are appropriate for the migration—
CSSPropertiesfor type-safe dimensions andcnutility for composing Tailwind classes.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/MyTransactions/Modal/ProposeSettlementModal/AmountField.tsx (1)
16-27: Critical: Type and unit mismatch in validation comparison.Line 20 compares
transactionAmount(a number in ETH units) withproposedAmount(a bigint in wei units). This comparison is invalid because:
parseFloat(amount)on Line 17 returns a number (e.g., 1.5 for 1.5 ETH)parseEther(amountProposed)on Line 18 returns a bigint in wei (e.g., 1500000000000000000n for 1.5 ETH)JavaScript will attempt numeric coercion but the units are fundamentally incompatible, leading to incorrect validation results.
🐛 Proposed fix: Convert both values to the same unit
useEffect(() => { - const transactionAmount = parseFloat(amount); - const proposedAmount = parseEther(amountProposed); + const transactionAmountWei = parseEther(amount); + const proposedAmountWei = parseEther(amountProposed); - if (amountProposed && transactionAmount < proposedAmount) { + if (amountProposed && transactionAmountWei < proposedAmountWei) { setError("Proposed amount exceeds transaction amount"); setIsAmountValid(false); } else { setError(""); setIsAmountValid(true); } }, [amountProposed, amount, setIsAmountValid]);
🧹 Nitpick comments (1)
web/src/pages/NewTransaction/Terms/Payment/TokenTransaction/TokenAndAmount/index.tsx (1)
21-28: Consider extracting token options to a constant.The token items array is hardcoded in the JSX. For better maintainability, consider extracting it to a constant at the file or module level.
♻️ Proposed refactor
+const TOKEN_OPTIONS = [ + { id: "xDAI", text: "xDAI", dot: "red" as const, itemValue: "xDAI" }, + { id: "ETH", text: "ETH", dot: "blue" as const, itemValue: "ETH" }, +]; + const TokenAndAmount: React.FC<ITokenAndAmount> = ({ quantity, setQuantity, token, setToken }) => { return ( <div className="flex flex-col gap-6 items-center mb-fluid-24-18 lg:flex-row"> <BigNumberField aria-label="Amount" placeholder="eg. 3.6" value={quantity || "0"} minValue="0" onChange={(value) => setQuantity(value.toString())} /> <DropdownSelect defaultSelectedKey={token} - items={[ - { id: "xDAI", text: "xDAI", dot: "red", itemValue: "xDAI" }, - { id: "ETH", text: "ETH", dot: "blue", itemValue: "ETH" }, - ]} + items={TOKEN_OPTIONS} callback={(value) => setToken(value.itemValue)} /> </div> ); };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/pages/MyTransactions/Modal/ProposeSettlementModal/AmountField.tsxweb/src/pages/NewTransaction/Terms/Payment/GeneralTransaction/TokenAndAmount/index.tsxweb/src/pages/NewTransaction/Terms/Payment/TokenTransaction/TokenAndAmount/index.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-28T13:58:33.048Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx:44-46
Timestamp: 2024-11-28T13:58:33.048Z
Learning: In `web/src/pages/NewTransaction/NavigationButtons/NextButton.tsx`, when checking `user.emailUpdateableAt`, it's acceptable to use the non-null assertion operator `!` due to type assertion issues preventing standard checks.
Applied to files:
web/src/pages/MyTransactions/Modal/ProposeSettlementModal/AmountField.tsxweb/src/pages/NewTransaction/Terms/Payment/GeneralTransaction/TokenAndAmount/index.tsxweb/src/pages/NewTransaction/Terms/Payment/TokenTransaction/TokenAndAmount/index.tsx
📚 Learning: 2024-10-08T16:23:56.291Z
Learnt from: kemuru
Repo: kleros/escrow-v2 PR: 60
File: web/src/pages/NewTransaction/NavigationButtons/DepositPaymentButton.tsx:63-64
Timestamp: 2024-10-08T16:23:56.291Z
Learning: Errors in the `useContractRead` hook for ERC20 token allowance checks in the `DepositPaymentButton` component are managed by the `wrapWithToast()` function, which logs errors in a notification popup. Additionally, the quantity for the approval is pre-validated and stored in the context of the NewTransactionContext.
Applied to files:
web/src/pages/NewTransaction/Terms/Payment/GeneralTransaction/TokenAndAmount/index.tsxweb/src/pages/NewTransaction/Terms/Payment/TokenTransaction/TokenAndAmount/index.tsx
⏰ 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). (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (8)
web/src/pages/MyTransactions/Modal/ProposeSettlementModal/AmountField.tsx (3)
1-4: LGTM!The import statements correctly reflect the migration to BigNumberField and the use of viem utilities.
6-10: LGTM!The interface properly captures the new validation callback prop.
29-41: No action needed. TheonChangehandler on line 34 correctly usesvalue.toString(). This pattern is consistently used across multiple BigNumberField instances throughout the codebase (TokenTransaction/TokenAndAmount, GeneralTransaction/TokenAndAmount, and ProposeSettlementModal/AmountField), confirming that the library's onChange callback provides a value object with atoString()method.web/src/pages/NewTransaction/Terms/Payment/GeneralTransaction/TokenAndAmount/index.tsx (3)
1-20: LGTM!The imports properly reflect the migration to BigNumberField and maintain existing wagmi hooks for balance checking.
43-66: LGTM!The validation logic correctly compares balance and entered amounts using consistent float types and units, avoiding the type mismatch issues present in other files.
73-91: Pattern is consistently used across the codebase and aligns with setter signatures, but official API verification is inconclusive.The
onChangehandler pattern usingvalue.toString()is consistently applied across all three BigNumberField implementations in the codebase (ProposeSettlementModal, GeneralTransaction TokenAndAmount, and TokenTransaction TokenAndAmount). The setter functions all expect string parameters, which aligns with this pattern. However, definitive verification against the @kleros/ui-components-library v3.6.0 API specification requires access to the library's official documentation, which is not publicly available through standard searches. If possible, review the library's TypeScript definitions or source code in your project's node_modules to confirm theonChangecallback parameter type.web/src/pages/NewTransaction/Terms/Payment/TokenTransaction/TokenAndAmount/index.tsx (2)
1-9: LGTM!The imports and interface properly capture the component's token selection and amount input requirements.
14-20: BigNumberField usage follows established pattern.The implementation correctly uses the same
value.toString()pattern as the other TokenAndAmount components. Verification of the API compatibility is already requested in previous files.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/src/components/StyledIcons/LawBalanceIcon.tsx (1)
4-6: Consider adding accessibility attributes to the SVG icon.For improved accessibility, consider adding either
aria-labelif the icon conveys semantic meaning, oraria-hidden="true"if it's purely decorative. This helps screen reader users understand the purpose of the icon.♿ Proposed accessibility enhancement
If the icon is decorative (e.g., accompanying text):
- return <LawBalance className="fill-klerosUIComponentsSecondaryPurple" />; + return <LawBalance className="fill-klerosUIComponentsSecondaryPurple" aria-hidden="true" />;If the icon conveys meaning (standalone):
- return <LawBalance className="fill-klerosUIComponentsSecondaryPurple" />; + return <LawBalance className="fill-klerosUIComponentsSecondaryPurple" aria-label="Law balance" role="img" />;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/components/StyledIcons/CheckCircleOutlineIcon.tsxweb/src/components/StyledIcons/LawBalanceIcon.tsxweb/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/ViewCaseButton.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- web/src/pages/MyTransactions/TransactionDetails/PreviewCardButtons/ViewCaseButton.tsx
⏰ 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). (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (2)
web/src/components/StyledIcons/CheckCircleOutlineIcon.tsx (1)
4-6: No issues found.The component is correctly implemented. The custom Tailwind class
fill-klerosUIComponentsSuccessis properly configured and used throughout the codebase (e.g., in StatusBanner.tsx, AccountDisplay.tsx, General.tsx), confirming the configuration is in place. The straightforward implementation without props is appropriate for this component's single, well-defined use case.web/src/components/StyledIcons/LawBalanceIcon.tsx (1)
5-5: No changes needed. The custom Tailwind color classfill-klerosUIComponentsSecondaryPurpleis correctly defined through the imported@kleros/ui-components-librarytheme (referenced inglobal.css), and the component applies it properly. This pattern is used consistently across multiple other components in the codebase.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @web/src/global.css:
- Around line 1-6: Move all @import rules to the top of web/src/global.css,
remove the redundant legacy directive @tailwind utilities, and replace the mixed
approach by keeping a single Tailwind v4 import @import "tailwindcss"; ensure
the UI library import (@import
"../../node_modules/@kleros/ui-components-library/dist/assets/theme.css") and
the @import "tailwindcss" appear before any other at-rules, and retain the
@custom-variant dark (&:where(.dark, .dark *)); line after imports.
In @web/src/pages/Settings/EmailConfirmation/index.tsx:
- Line 126: The decorative Icon component rendering at the Icon JSX (Icon
width={250} height={250} ...) should be hidden on small/mobile viewports and
only shown on desktop: update the Icon usage (the Icon element) to apply
responsive visibility classes or styles (e.g., hide by default and show at the
desired breakpoint like sm/lg) so it does not render on narrow screens and cause
overflow; ensure the responsive class matches your CSS framework breakpoints
(e.g., hidden sm:block or equivalent) or wrap it in a container with a
media-query-based display rule to only show on desktop.
🧹 Nitpick comments (1)
web/src/pages/Settings/EmailConfirmation/index.tsx (1)
12-17: Consider using a string enum for better debugging.The numeric enum works fine, but a string enum would provide better debugging output in logs and dev tools.
♻️ Optional refactor to string enum
-enum TextColor { - Primary, - Error, - Success, - Warning, -} +enum TextColor { + Primary = "primary", + Error = "error", + Success = "success", + Warning = "warning", +}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/src/global.cssweb/src/pages/Settings/EmailConfirmation/index.tsxweb/src/pages/Settings/index.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-28T14:07:11.735Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/layout/Header/navbar/index.tsx:56-56
Timestamp: 2024-11-28T14:07:11.735Z
Learning: In `web/src/layout/Header/navbar/index.tsx`, it's acceptable not to pass the optional `initialTab` prop when rendering the `Settings` component, as the component handles it with a fallback value.
Applied to files:
web/src/pages/Settings/index.tsx
📚 Learning: 2024-11-28T14:13:54.217Z
Learnt from: Harman-singh-waraich
Repo: kleros/escrow-v2 PR: 86
File: web/src/pages/Settings/EmailConfirmation/index.tsx:141-153
Timestamp: 2024-11-28T14:13:54.217Z
Learning: In the `EmailConfirmation` component, adding a cleanup function with an unsubscribed variable inside `useEffect` is unnecessary unless there's a specific need to handle component unmounting or prevent memory leaks.
Applied to files:
web/src/pages/Settings/EmailConfirmation/index.tsx
🧬 Code graph analysis (1)
web/src/pages/Settings/EmailConfirmation/index.tsx (1)
web/src/utils/index.ts (1)
cn(41-43)
🪛 Biome (2.1.2)
web/src/global.css
[error] 4-4: This @import is in the wrong position.
Any @import rules must precede all other valid at-rules and style rules in a stylesheet (ignoring @charset and @layer), or else the @import rule is invalid.
Consider moving import position.
(lint/correctness/noInvalidPositionAtImportRule)
⏰ 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). (6)
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Redirect rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Header rules - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
- GitHub Check: Pages changed - kleros-escrow-v2
🔇 Additional comments (9)
web/src/pages/Settings/EmailConfirmation/index.tsx (3)
119-121: Verify subtitle color behavior is intentional.The subtitle uses a fixed
text-klerosUIComponentsPrimaryTextcolor while the header (line 118) dynamically changes color based on the message type. Confirm this is the intended design, as users might expect both to share the same semantic color.
105-129: Excellent migration to Tailwind CSS!The refactoring successfully replaces styled-components with Tailwind utilities while maintaining functionality and improving semantic HTML structure. The responsive layout implementation is clean and follows mobile-first best practices.
21-33: The custom Tailwind colors are properly defined and configured. They are imported from the@kleros/ui-components-librarypackage via the@importstatement inglobal.css, and the@sourcedirective makes them available to Tailwind. No action needed—the color mappings in lines 21-33 are correctly set up and working throughout the codebase.web/src/pages/Settings/index.tsx (2)
4-4: LGTM: clsx import supports the new utility-class pattern.The addition of
clsxaligns with the Tailwind migration strategy for composing utility classes.
8-13: No changes needed. Custom utilities are properly configured.The project uses Tailwind v4 (
^4.1.17) with CSS variables defined in the@themeblock ofglobal.css. In Tailwind v4, CSS variables automatically generate corresponding utility classes, so:
--max-width-landscape→max-w-landscape--spacing-fluid-24-136→px-fluid-24-136--spacing-fluid-32-80→pt-fluid-32-80The className syntax used in lines 8-13 is correct and requires no refactoring. The arbitrary value syntax suggested in the original review is unnecessary for this Tailwind v4 setup with
@themedeclarations.Likely an incorrect or invalid review comment.
web/src/global.css (4)
7-63: Custom theme tokens require utility class mapping.The
@themeblock correctly defines custom CSS variables for colors, spacing, and other design tokens using Tailwind v4 syntax. The fluid spacing tokens usingclamp()provide excellent responsive behavior.However, these CSS variables alone don't automatically create utility classes. The Settings component uses classes like
px-fluid-24-136which need to map to--spacing-fluid-24-136. Ensure your Tailwind configuration includes plugins or theme extensions to expose these as utility classes, or use arbitrary value syntax as noted in the Settings review.
65-68: LGTM: Dark mode skeleton color overrides.The dark mode color adjustments for skeleton loading are correctly scoped to the
.darkclass, aligning with the custom dark variant defined earlier.
83-124: LGTM: Base styles and typography.The base styles correctly establish typography, box-sizing reset, and element styling using CSS variables from the imported UI component library theme. The approach ensures consistency across the application.
125-163: LGTM: Utility classes for third-party components.The custom utility classes appropriately handle:
- Custom scrollbar theming with interactive states (hover, active)
- Toastify positioning adjustments
- PDF viewer canvas element hiding
The implementation is clean and properly scoped.
| </ButtonContainer> | ||
| </WarningModal> | ||
| <Overlay> | ||
| <StyledModal className="w-[400px] max-w-[90vw] gap-4 items-center" ref={modalRef}> |
There was a problem hiding this comment.
I think we should replace it with the Modal Component from the library now
There was a problem hiding this comment.
I agree, but I didn't do it because I don't think this should be done on this PR, as it is not really about the migration and it would bloat it.
I think a separate PR to both remove the react-modal dependency and also change every modal to use the component from the library is better.
What do you think?
| <br /> | ||
| <StyledP> | ||
| <div className="flex flex-col gap-4"> | ||
| <div className="text-klerosUIComponentsPrimaryText m-0 wrap-break-word"> |
There was a problem hiding this comment.
Originally, this was indeed a p, but it shouldn't as it has div's as children.
Particularly, the Copiable element renders div's, so we get a lot of warnings, that's why I changed it. I think div is the best approach.
Resolves #127.
PR-Codex overview
This PR focuses on refactoring the codebase by removing unused styles and components, updating button props, and migrating to
tailwindcssfor styling. It enhances code readability and maintainability while ensuring consistent styling across the application.Detailed summary
onClickwithonPressin button components for consistency.LG_BREAKPOINT.aria-labelattributes to inputs.Summary by CodeRabbit
Styling & Theme System
UI Improvements
UI Library Upgrade
Chores
✏️ Tip: You can customize this high-level summary in your review settings.