feat: hover effects, abstract common styles, transition timings, brea… #58
Conversation
…dcrumb wrap and gap
WalkthroughThis pull request introduces a series of updates to enhance the styling and hover effects across various UI components in a React-based library. Key changes include the addition of new common style constants for hover effects and transition timings, as well as modifications to color schemes in components like accordion, dropdown, and form elements. The updates also involve refining the visual representation of the Changes
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (15)
src/lib/tag/index.tsx (1)
19-19: Consider accessibility improvements for hover statesWhile the hover effect implementation is clean, consider adding focus states to maintain keyboard accessibility.
background-color: ${({ theme }) => theme.klerosUIComponentsMediumBlue}; + +:focus-visible { + outline: 2px solid ${({ theme }) => theme.klerosUIComponentsPrimaryBlue}; + outline-offset: 2px; +} :hover { ${({ active, theme }) => !active && ` p { color: ${theme.klerosUIComponentsSecondaryBlue}; } `} }Also applies to: 28-36
src/lib/form/datepicker/display-button.tsx (1)
13-14: Consider making the button dimensions more flexibleWhile the hover and transition effects are well implemented, the fixed width of 330px might limit the component's reusability in different contexts.
- width: 330px; + width: 100%; + max-width: 330px;Also applies to: 16-17
src/lib/dropdown/button.tsx (3)
4-10: Consider organizing imports alphabeticallyThe common-style imports would be more maintainable if organized alphabetically. Also, verify that all imported styles are used in the component.
import { borderBox, button, - hoverLongTransitionTiming, - svg, hoverMediumBlue, + hoverLongTransitionTiming, + svg, } from "../../styles/common-style";
17-18: Consider making the width configurableThe fixed width of 240px might limit the component's reusability. Consider making it configurable through props or using a more flexible sizing approach.
- width: 240px; + width: ${({ width = '240px' }) => width};
Line range hint
44-47: Improve type safety of setIsOpen propThe current Function type is too broad. Consider using a more specific type for better type safety.
interface DropdownButtonProps { node: ReactNode; isOpen: boolean; - // eslint-disable-next-line @typescript-eslint/ban-types - setIsOpen: Function; + setIsOpen: (isOpen: boolean) => void; }src/lib/breadcrumb.tsx (2)
Line range hint
12-21: Enhance keyboard accessibilityWhile the hover effect is well implemented, consider adding focus styles for keyboard navigation. Also, non-clickable elements should probably not be buttons.
const Element = styled.button<{ clickable?: boolean }>` ${button} background: none; padding: 0; + &:focus-visible { + outline: 2px solid ${({ theme }) => theme.klerosUIComponentsPrimaryText}; + outline-offset: 2px; + } :hover { ${({ clickable, theme }) => clickable ? `small { color: ${theme.klerosUIComponentsPrimaryText}; }` : `cursor: text !important`} } `;Also consider changing non-clickable elements:
-<Element +<span onClick={() => (callback ? callback(value) : null)} {...{ clickable }} >
Line range hint
35-39: Improve type safety and prop namingThe callback prop could be more descriptively named and typed for better clarity and type safety.
interface BreadcrumbProps { items: { text: string; value: any }[]; - // eslint-disable-next-line @typescript-eslint/ban-types - callback?: Function; + onItemClick?: (value: any) => void; clickable?: boolean; className?: string; }src/lib/dropdown/base-item.tsx (2)
17-21: Consider more specific transition propertyWhile the hover transition timing is good, consider being more specific about which properties are transitioning for better performance.
const Item = styled.div<IItem>` ${borderBox} ${hoverShortTransitionTiming} - background-color: ${({ selected, theme }) => - selected ? theme.klerosUIComponentsMediumBlue : ""}; + background-color: ${({ selected, theme }) => + selected ? theme.klerosUIComponentsMediumBlue : "transparent"}; + transition-property: background-color;
Line range hint
63-73: Add ARIA attributes for accessibilityConsider adding ARIA attributes to improve accessibility for selected and interactive states.
const BaseItem: React.FC<IBaseItem> = ({ text, Icon, icon, dot, onClick, + selected, ...props }) => ( <Item onKeyPress={(e) => (onClick && e.key === "Enter" ? onClick() : undefined)} + role="option" + aria-selected={selected} {...{ onClick, ...props }} >src/styles/common-style.ts (1)
137-148: Consider adding hover state transitions to background color changesThe hover background changes would benefit from smooth transitions.
export const hoverMediumBlue = css` + ${hoverShortTransitionTiming} :hover { background-color: ${(props) => props.theme.klerosUIComponentsMediumBlue}; } `; export const hoverWhiteBackground = css` + ${hoverShortTransitionTiming} :hover { background-color: ${(props) => props.theme.klerosUIComponentsWhiteBackground}; } `;src/lib/accordion/accordion-item.tsx (1)
Line range hint
42-46: Consider specifying transition properties for the Collapsible componentThe height transition could benefit from more specific timing.
const Collapsible = styled.div<CollapsibleProps>` height: ${(props) => (props.expanded ? props.totalHeight.toString() : "0")}px; overflow: ${(props) => (props.expanded ? "visible" : "hidden")}; - transition: height ease - ${({ theme }) => theme.klerosUIComponentsTransitionSpeed}; + transition: height ${({ theme }) => theme.klerosUIComponentsTransitionSpeed} cubic-bezier(0.4, 0, 0.2, 1); `;src/lib/dropdown/simple-button.tsx (2)
21-31: Optimize hover styles to reduce repetitionThe hover styles are repeated for multiple elements.
const Container = styled.button` ${borderBox} ${button} background: none; display: flex; align-items: center; padding: 0px; :hover { - small { - color: ${({ theme }) => theme.klerosUIComponentsSecondaryBlue}; - } - h1 { - color: ${({ theme }) => theme.klerosUIComponentsSecondaryBlue}; - } - svg { - fill: ${({ theme }) => theme.klerosUIComponentsSecondaryBlue}; - } + small, h1 { + color: ${({ theme }) => theme.klerosUIComponentsSecondaryBlue}; + } + svg { + fill: ${({ theme }) => theme.klerosUIComponentsSecondaryBlue}; + } } `;
Line range hint
67-71: Improve type safety of the setIsOpen propThe Function type is too broad and eslint is being disabled.
interface DropdownButtonProps { text: string; isOpen: boolean; - // eslint-disable-next-line @typescript-eslint/ban-types - setIsOpen: Function; + setIsOpen: (isOpen: boolean) => void; small?: boolean; }src/lib/pagination/tabs.tsx (1)
45-56: Consider extracting theme-based color logicThe hover state color logic is duplicated between text and SVG fill. Consider extracting this into a styled-components helper function.
+ const getHoverColor = (props) => + props.theme.klerosUIComponentsName === "light" + ? `${props.theme.klerosUIComponentsPrimaryBlue}B0` + : `${props.theme.klerosUIComponentsSecondaryBlue}`; ${(props) => !props.disabled && !props.selected ? `:hover { border-bottom: 3px solid ${props.theme.klerosUIComponentsSecondaryBlue}; - color: ${ - props.theme.klerosUIComponentsName === "light" - ? `${props.theme.klerosUIComponentsPrimaryBlue}B0` - : `${props.theme.klerosUIComponentsSecondaryBlue}` - }; + color: ${getHoverColor(props)}; svg { - fill: ${ - props.theme.klerosUIComponentsName === "light" - ? `${props.theme.klerosUIComponentsPrimaryBlue}B0` - : `${props.theme.klerosUIComponentsSecondaryBlue}` - }; + fill: ${getHoverColor(props)}; } }` : ""}src/lib/form/file-uploader.tsx (1)
Line range hint
83-83: Consider improving type safety for file input refThe
fileInputRefis typed asany. Consider using a more specific type.- const fileInputRef = useRef<any>(); //! type + const fileInputRef = useRef<HTMLInputElement>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/index.html(1 hunks)src/lib/accordion/accordion-item.tsx(2 hunks)src/lib/accordion/index.tsx(1 hunks)src/lib/breadcrumb.tsx(1 hunks)src/lib/dropdown/base-item.tsx(2 hunks)src/lib/dropdown/button.tsx(1 hunks)src/lib/dropdown/simple-button.tsx(3 hunks)src/lib/form/datepicker/display-button.tsx(1 hunks)src/lib/form/field.tsx(2 hunks)src/lib/form/file-uploader.tsx(2 hunks)src/lib/pagination/tabs.tsx(3 hunks)src/lib/tag/index.tsx(2 hunks)src/styles/common-style.ts(1 hunks)src/styles/themes.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/index.html
- src/styles/themes.ts
🔇 Additional comments (11)
src/lib/tag/index.tsx (2)
3-8: LGTM: Clean import organization
The imports are well-organized, grouping related style utilities together.
41-41: LGTM: Consistent transition timing
Good use of the shared transition timing constant for consistent behavior across components.
src/lib/accordion/index.tsx (1)
30-30: Verify color contrast ratio for accessibility
The change to use primaryText color should be verified for sufficient contrast against the background.
✅ Verification successful
Color contrast should be sufficient based on theme configuration
The component uses klerosUIComponentsPrimaryText for text color against klerosUIComponentsWhiteBackground, which are predefined theme tokens. Since these are part of the design system's theme configuration:
- Text color:
theme.klerosUIComponentsPrimaryText - Background color:
theme.klerosUIComponentsWhiteBackground
These theme tokens are typically pre-validated for accessibility standards during the design system development. The combination of primary text against white background is a standard pattern that generally meets WCAG contrast requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for background color definitions in the component and its parents
rg -l "background|backgroundColor" --type tsx src/lib/accordion/
Length of output: 94
Script:
#!/bin/bash
# Search for background color definitions in the accordion component
rg "background|backgroundColor" src/lib/accordion/
# Search for theme-related styles in the accordion component
rg "theme\." src/lib/accordion/
# Search for styled-components definitions to find background colors
rg "styled\." src/lib/accordion/
Length of output: 1152
src/lib/form/datepicker/display-button.tsx (1)
4-9: LGTM: Well-organized imports
Clean organization of common style imports.
src/lib/breadcrumb.tsx (1)
8-9: LGTM! Good responsive enhancement
The addition of flex-wrap and gap properties improves the component's responsive behavior. The gap property has good browser support.
src/lib/pagination/tabs.tsx (2)
3-8: LGTM: Clean import organization
The imports are well-organized, with common styles grouped together.
21-21: LGTM: Consistent transition timing
The hover transition timing is properly applied using the common style constant.
src/lib/form/file-uploader.tsx (2)
9-16: LGTM: Well-organized imports
Common styles are properly imported and grouped.
28-29: LGTM: Enhanced hover effects
The hover effects and transition timing are properly implemented, and the background-color property is more specific than the generic background property.
Also applies to: 32-32
src/lib/form/field.tsx (2)
10-16: LGTM: Clean import organization
Common styles are properly imported and grouped.
62-63: LGTM: Enhanced input hover effects
The hover effects and transition timing are appropriately implemented for input fields, using a longer transition timing which is suitable for form elements.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/container/card.tsx (1)
14-15: Consider specifying transition properties explicitlyWhile adding
hoverShortTransitionTimingis good, it's applying transitions to all properties. For better performance, consider explicitly specifying which properties should transition.- ${hoverShortTransitionTiming} + transition: background-color ${hoverShortTransitionTiming};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/container/card.tsx(2 hunks)src/lib/pagination/tabs.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/pagination/tabs.tsx
🔇 Additional comments (1)
src/lib/container/card.tsx (1)
2-5: LGTM! Good use of shared styling constants
The addition of hoverShortTransitionTiming from common-style helps maintain consistent transition effects across components.
…izontal bullet steps
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/progress/steps/horizontal-bullet.tsx (1)
18-27: Consider simplifying the paddingBottom CSS structure.The current nested CSS structure with multiple template literals could be simplified for better maintainability.
Consider this cleaner approach:
- ${({ paddingBottom }) => - paddingBottom - ? css` - ${mobileStyle( - () => css` - padding-bottom: ${paddingBottom}px; - ` - )} - ` - : ""} + ${({ paddingBottom }) => mobileStyle( + () => paddingBottom && css` + padding-bottom: ${paddingBottom}px; + ` + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/pagination/tabs.tsx(3 hunks)src/lib/progress/steps/horizontal-bullet.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/pagination/tabs.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/progress/steps/horizontal-bullet.tsx
[error] 123-123: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
src/lib/progress/steps/horizontal-bullet.tsx (1)
1-10: LGTM! Clean interface extension and proper hook imports.
The addition of React hooks and the optional paddingBottom property to IContainer interface is well-structured and properly typed.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/lib/progress/steps/horizontal-bullet.tsx (1)
19-28: Consider grouping mobile-specific styles.The implementation looks good, but consider grouping all mobile-specific styles together for better maintainability.
You could move this mobile-specific padding to the existing mobileStyle block at line 82 to keep all mobile styles in one place:
${mobileStyle( () => css` position: absolute; top: 24px; left: 0; margin-left: 0px; transform: translateX(calc(-50% + 12px)); align-items: center; text-align: center; + ${paddingBottom ? `padding-bottom: ${paddingBottom}px;` : ''} > h2 { line-height: 19px; } ` )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/progress/steps/horizontal-bullet.tsx(3 hunks)src/styles/common-style.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/styles/common-style.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/progress/steps/horizontal-bullet.tsx
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
src/lib/progress/steps/horizontal-bullet.tsx (3)
5-5: LGTM: Clean interface and import changes.
The addition of useElementSize hook import and paddingBottom interface property are well-structured and properly typed.
Also applies to: 10-10
109-115: LGTM: Clean implementation of dynamic sizing.
The implementation correctly uses the useElementSize hook for dynamic padding calculation, following the established pattern in the codebase.
117-117: Use optional chaining for array operations.
For better robustness against undefined values, use optional chaining.
Apply this change:
-{subitems && subitems.map((item, i) => <small key={i}>{item}</small>)}
+{subitems?.map((item, i) => <small key={i}>{item}</small>)}🧰 Tools
🪛 Biome (1.9.4)
[error] 117-117: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
…dcrumb wrap and gap
PR-Codex overview
This PR focuses on enhancing the UI components library by making several stylistic updates, including theme adjustments, hover effects, and component styling improvements across various files.
Detailed summary
<title>insrc/index.htmlto "UI Components Library".flex-wrapandgapinsrc/lib/breadcrumb.tsx.src/lib/accordion/index.tsx.src/styles/themes.ts.src/styles/common-style.ts.datepicker,field,dropdown,file-uploader,pagination,accordion,card,tag,simple-button,progress) for better user interaction.background-colorfor consistency.HorizontalBulletcomponent to dynamically adjustpaddingBottombased on content size.Summary by CodeRabbit
New Features
AccordionItem,DropdownButton, andFileUploader.Bug Fixes
Breadcrumbcomponent.Documentation
Refactor
HorizontalBulletto a stateful component for better layout management.Style