add size for kbd component #942
Conversation
WalkthroughThis pull request enhances the Changes
Sequence Diagram(s)sequenceDiagram
participant Story as Size Story
participant Kbd as Kbd Component
participant DOM as Rendered Element
participant CSS as CSS Styles
Story->>Kbd: Render <kbd> with size prop
Kbd->>DOM: Return <kbd data-kbd-size="value">
DOM->>CSS: Apply styles based on data attribute
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (3)
styles/themes/components/kbd.scss (1)
29-52: Well-structured size variants with a minor improvement suggestionThe implementation of size variants is clean and follows a logical progression. The commented section header makes the purpose clear.
Consider using specific units (px, rem) for font-size instead of keywords (small, medium, large, x-large) to ensure more consistent cross-browser rendering:
&[data-kbd-size="small"]{ - font-size: small; + font-size: 0.875rem; /* or 14px */ padding: 2px 8px; height: 24px; } &[data-kbd-size="medium"]{ - font-size: medium; + font-size: 1rem; /* or 16px */ padding: 4px 12px; height: 32px; } /* And so on for other sizes */src/components/ui/Kbd/Kbd.tsx (2)
5-5: Remove unused importThe
useCreateDataAttributehook is imported but not used in the component.-import { useCreateDataAttribute } from '~/core/hooks/createDataAttribute';🧰 Tools
🪛 GitHub Check: lint
[warning] 5-5:
'useCreateDataAttribute' is defined but never used
16-16: Fix spacing in default parameter valueThere's a spacing issue with the default parameter according to your linting rules.
-const Kbd = ({ children, customRootClass, className, size= '', ...props }: KbdProps) => { +const Kbd = ({ children, customRootClass, className, size = '', ...props }: KbdProps) => {🧰 Tools
🪛 ESLint
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/ui/Kbd/Kbd.tsx(1 hunks)src/components/ui/Kbd/stories/Kbd.stories.tsx(2 hunks)styles/themes/components/kbd.scss(1 hunks)
🧰 Additional context used
🪛 ESLint
src/components/ui/Kbd/Kbd.tsx
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
🪛 GitHub Check: lint
src/components/ui/Kbd/Kbd.tsx
[warning] 5-5:
'useCreateDataAttribute' is defined but never used
🔇 Additional comments (5)
src/components/ui/Kbd/stories/Kbd.stories.tsx (2)
5-5: Good addition of size constantsThe array of size options is well-defined and consistent with the implementation in the styles.
25-40: Well-structured size demonstrationThe Size story effectively demonstrates all available size variants in an organized layout, providing a clear visual reference for the new feature.
src/components/ui/Kbd/Kbd.tsx (3)
12-13: Size property added correctlyThe size property is correctly defined as an optional string in the KbdProps type.
13-13: Review props type definitionThe
propsproperty is typed as an array of Records, which seems unusual. Typically, this would be a single Record object for spreading additional props onto the element.- props: Record<string, any>[]; + props?: Record<string, any>;Please verify if this change is correct for your component pattern.
19-23: Good implementation of conditional data attributeThe conditional addition of the data attribute when size is provided is clean and effective.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/ui/Kbd/Kbd.tsx (1)
16-16: Fix spacing around the default value assignment.There should be spaces around the equals sign for the default value assignment.
-const Kbd = ({ children, customRootClass, className, size= '', ...props }: KbdProps) => { +const Kbd = ({ children, customRootClass, className, size = '', ...props }: KbdProps) => {🧰 Tools
🪛 ESLint
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ui/Kbd/Kbd.tsx(1 hunks)
🧰 Additional context used
🪛 ESLint
src/components/ui/Kbd/Kbd.tsx
[error] 16-16: Operator '=' must be spaced.
(space-infix-ops)
🔇 Additional comments (4)
src/components/ui/Kbd/Kbd.tsx (4)
5-5: Good addition of the useCreateDataAttribute hook.Clean import of the necessary hook to support the new size functionality.
12-12: Properly typed the new size prop.The optional
sizeprop is correctly defined with TypeScript's optional property syntax.
19-19: Good use of the useCreateDataAttribute hook.This is an effective way to create the data attribute for the size property.
21-21: Correctly applied data attributes to the kbd element.The data attributes are properly spread into the JSX element along with other props.
kotAPI
added
the
automerge
3 participants
kbd component
Summary by CodeRabbit
New Features
Documentation