-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(stepper): added stepper component #1500
Conversation
WalkthroughA new Stepper component has been developed for the React component library, introducing a visual representation of step progression. The implementation includes a TypeScript-based component with SCSS styling, Storybook stories for documentation, and comprehensive unit tests to ensure robust functionality. Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/react-components/src/components/Stepper/Stepper.module.scss (3)
1-2
: Consider adding a namespace prefix to prevent class name collisions.-$base-class: 'stepper'; +$base-class: 'ds-stepper';
12-15
: Consider mobile responsiveness.Add media queries to handle overflow on small screens. Consider scrolling or wrapping behavior.
&__steps__container { display: flex; flex-flow: row nowrap; + overflow-x: auto; + -webkit-overflow-scrolling: touch; + scrollbar-width: none; + &::-webkit-scrollbar { + display: none; + } }
20-21
: Use CSS custom properties for step dimensions.Extract hardcoded values to custom properties for better maintainability.
- width: 22px; - height: 4px; + width: var(--stepper-step-width, 22px); + height: var(--stepper-step-height, 4px);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/react-components/src/components/Stepper/Stepper.module.scss
(1 hunks)packages/react-components/src/components/Stepper/Stepper.stories.tsx
(1 hunks)packages/react-components/src/components/Stepper/Stepper.test.tsx
(1 hunks)packages/react-components/src/components/Stepper/Stepper.tsx
(1 hunks)packages/react-components/src/components/Stepper/index.ts
(1 hunks)packages/react-components/src/components/Stepper/types.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/react-components/src/components/Stepper/index.ts
🧰 Additional context used
🪛 GitHub Actions: Tests
packages/react-components/src/components/Stepper/Stepper.test.tsx
[error] 10-10: Test failure: Unable to find element with data-testid='stepper'. The component is rendered but missing the required test ID attribute.
🔇 Additional comments (2)
packages/react-components/src/components/Stepper/types.ts (1)
3-19
: Interface looks solid!
Well-defined props with clear docstrings.packages/react-components/src/components/Stepper/Stepper.stories.tsx (1)
1-60
: Great Storybook coverage!
Nicely demonstrates various states and sizes.
packages/react-components/src/components/Stepper/Stepper.test.tsx
Outdated
Show resolved
Hide resolved
color: var(--content-basic-disabled); | ||
font-weight: var(--font-weight-medium); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Color contrast ratios are insufficient for accessibility
The disabled text color fails WCAG AA requirements in both themes:
- Light theme: 3.39:1 (#8d8d95 on white)
- Dark theme: 3.16:1 (#767680 on #202024)
Minimum required contrast ratio for normal text is 4.5:1.
🔗 Analysis chain
Verify color contrast for counter text.
Ensure that --content-basic-disabled
provides sufficient contrast against the background for accessibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the color variable definition
rg --type css "content-basic-disabled" -A 2
Length of output: 10416
Script:
#!/bin/bash
# Search for background color definitions in Stepper component
rg -B 2 -A 2 "background|surface" packages/react-components/src/components/Stepper/Stepper.module.scss
# Also search theme files for default surface colors
rg "surface-primary-default|surface-basic-default" packages/react-components/src/themes/
Length of output: 2032
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use another one for those step number 1/3...
--Content-Basic-Placeholder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vladko-uxds changed ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/react-components/src/components/Stepper/Stepper.spec.tsx (2)
25-30
: Custom class functionality
Coverage is good. You could also test multiple custom classes if needed.
42-49
: Additional props coverage
Consider adding more props to verify broader attribute consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-components/src/components/Stepper/Stepper.spec.tsx
(1 hunks)packages/react-components/src/components/Stepper/Stepper.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-components/src/components/Stepper/Stepper.tsx
🔇 Additional comments (3)
packages/react-components/src/components/Stepper/Stepper.spec.tsx (3)
7-13
: Test coverage is robust
Thanks for verifying the step count thoroughly.
15-23
: Consider edge steps
Include a test whenactiveStep === steps
to check final step class assignment.
32-40
: Solid error handling
Your range validation test is well structured.
packages/react-components/src/components/Stepper/Stepper.module.scss
Outdated
Show resolved
Hide resolved
color: var(--content-basic-disabled); | ||
font-weight: var(--font-weight-medium); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/react-components/src/components/Stepper/Stepper.stories.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/src/components/Stepper/Stepper.spec.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/react-components/src/components/Stepper/Stepper.module.scss
(1 hunks)packages/react-components/src/components/Stepper/Stepper.spec.tsx
(1 hunks)packages/react-components/src/components/Stepper/Stepper.stories.tsx
(1 hunks)packages/react-components/src/components/Stepper/Stepper.tsx
(1 hunks)packages/react-components/src/index.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react-components/src/components/Stepper/Stepper.module.scss
- packages/react-components/src/components/Stepper/Stepper.spec.tsx
- packages/react-components/src/components/Stepper/Stepper.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: chromatic-deployment
🔇 Additional comments (2)
packages/react-components/src/index.ts (1)
65-65
: Export introduction looks good!
No issues spotted with adding the Stepper export here.packages/react-components/src/components/Stepper/Stepper.tsx (1)
13-19
: Kudos for including a default test ID!
This aligns with previous feedback and ensures tests run smoothly.
packages/react-components/src/components/Stepper/Stepper.spec.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change color token for stepper label
--Content-Basic-Placeholder
color: var(--content-basic-disabled); | ||
font-weight: var(--font-weight-medium); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use another one for those step number 1/3...
--Content-Basic-Placeholder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Resolves: #1496
Description
This pull request introduces a new
Stepper
component to thereact-components
package. The changes include the implementation of the component, its styles, tests, and stories for Storybook.Key changes:
Component Implementation:
packages/react-components/src/components/Stepper/Stepper.tsx
: Added theStepper
component with properties for active step, total steps, custom class names, and additional props. It includes logic to render steps and handle invalid active steps.Styling:
packages/react-components/src/components/Stepper/Stepper.module.scss
: Defined styles for theStepper
component, including base styles and modifiers for completed and active steps.Storybook:
packages/react-components/src/components/Stepper/Stepper.stories.tsx
: Added Storybook stories for theStepper
component, showcasing different states such as default, middle progress, completed, and custom sizes.Testing:
packages/react-components/src/components/Stepper/Stepper.test.tsx
: Added tests to verify the correct number of steps, the application of completed and active classes, custom class names, error handling for out-of-range active steps, and passing additional props.Type Definitions:
packages/react-components/src/components/Stepper/types.ts
: Defined theStepperProps
interface, specifying the properties required and optional for theStepper
componentStorybook
https://feature-1496--613a8e945a5665003a05113b.chromatic.com
Checklist
Obligatory:
Summary by CodeRabbit
New Features
Documentation
Style