feat: create Stepper component #2098
Conversation
bityutskiyAO
commented
Feb 7, 2025
bityutskiyAO
commented
- create Stepper component
- create stories for Stepper
- add README files for both languages (en, ru)
|
Preview is ready. |
|
Visual Tests Report is ready. |
bityutskiyAO
force-pushed
the
feat/create-stepper-component
branch
from
0a4369b to
9d1480e
Compare
src/components/Stepper/README.md
Outdated
| | qa | `data-qa` HTML attribute, used for testing. | `string` | | | ||
| | separator | Custom separator node. | `React.ReactNode` | | | ||
| | className | CSS class name for the element. | `string` | | | ||
| | style | Sets the inline style for the element. | `CSSProperties` | | |
There was a problem hiding this comment.
I think we may clarify that we are talking about a container
src/components/Stepper/Stepper.scss
Outdated
| background-color: var(--g-color-base-generic); | ||
| } | ||
|
|
||
| &__text { |
There was a problem hiding this comment.
That is not valid naming according to bem methodology. There we have &__item__text, block in a block. Could add it as a separate block?
src/components/Stepper/Stepper.scss
Outdated
| @include mixins.overflow-ellipsis(); | ||
| } | ||
|
|
||
| &__icon { |
There was a problem hiding this comment.
The same issue about block in a block
| @@ -0,0 +1,23 @@ | |||
| import type {StepperItemProps} from '../StepperItem'; | |||
|
|
|||
| export const useRelativeSteps = ( | |||
There was a problem hiding this comment.
It seems that this code doesn't used. Why we need it?
There was a problem hiding this comment.
When we talking about 2 types of steps "relative" and "non relative", we discuss, that it may be helpfull for end-users to have this hook for auto disabling steps after selected
There was a problem hiding this comment.
I think we can skip it for the first iteration because we don't know real user cases
| }; | ||
|
|
||
| return ( | ||
| <button |
There was a problem hiding this comment.
I think we can use gravity Button here
| separator?: React.ReactNode; | ||
| }; | ||
|
|
||
| export const StepperSeparator = ({separator}: StepperSeparatorProps) => { |
There was a problem hiding this comment.
We can use smth like BreadcrumbsSeparator
src/components/Stepper/Stepper.tsx
Outdated
| } | ||
|
|
||
| export const Stepper = (props: StepperProps) => { | ||
| const {children, value = 0, size = 's', className, onUpdate, separator} = props; |
There was a problem hiding this comment.
Is it possible to use component without initial value?
There was a problem hiding this comment.
u mean without value = 0? I guess, it can
There was a problem hiding this comment.
Yep. So, it looks like an uncontrolled state if we need to set value=undefined, otherwise we need some special value to have such behaviour in controlled state
src/components/Stepper/Stepper.tsx
Outdated
| }, [children, value, size, onUpdate, separator]); | ||
|
|
||
| return ( | ||
| <ol className={b(null, className)} style={props.style} data-qa={props.qa}> |
There was a problem hiding this comment.
Component can receive DOMProps and AriaLabelingProps, but u don't use it here. U can get Breadcrumbs as an example
src/components/Stepper/Stepper.tsx
Outdated
| separator?: React.ReactNode; | ||
| } | ||
|
|
||
| function Item(_props: StepperItemProps): React.ReactElement | null { |
There was a problem hiding this comment.
I think we don't need this component, so u can use StepperItem directly
There was a problem hiding this comment.
I try to remove it, but it usefull because of the fact, that i need to add some props to StepperItem (selected, onUpdate, size) that should not be directly accessible via StepItem (root component pass it)
src/components/Stepper/Stepper.tsx
Outdated
| const stepItems = React.useMemo(() => { | ||
| const items: React.ReactElement<StepperItemProps>[] = []; | ||
|
|
||
| React.Children.forEach(children, (child, index) => { |
There was a problem hiding this comment.
Do we actually need this logic? I think we can get all the children and StepperSeparator ti them without cloning them
src/components/Stepper/Stepper.tsx
Outdated
| export interface StepperProps extends DOMProps, AriaLabelingProps, QAProps { | ||
| children: React.ReactElement<StepperItemProps> | React.ReactElement<StepperItemProps>[]; | ||
| value?: number | string; | ||
| onUpdate: (id?: number | string) => void; |
There was a problem hiding this comment.
I make it required because of the fact, that without it consumer of the component doesn't have any information about current selected step, it become fully uncontrolled and selection logic won't work (we don't have any inner state for now)
There was a problem hiding this comment.
after all - i make it optional
src/components/Stepper/Stepper.tsx
Outdated
| Stepper.Item = Item; | ||
| Stepper.displayName = 'Stepper'; | ||
|
|
||
| export default Stepper; |
src/components/Stepper/Stepper.tsx
Outdated
|
|
||
| function Item(_props: StepperItemProps): React.ReactElement | null { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Why not using StepperItem inself and clone while mapping children?
| onClick={onClick} | ||
| disabled={disabled} | ||
| size={size} | ||
| view={selectedItem ? 'outlined-info' : 'outlined'} |
There was a problem hiding this comment.
Should it to be a brand color, not info?
There was a problem hiding this comment.
In figma we have --g-color-line-info for selected item, should it be changed to brand?
| const selectedItem = id === selected; | ||
|
|
||
| return ( | ||
| <Button |
There was a problem hiding this comment.
Let's proxy all button props, including all html attributes. It would be a nice feature to wrap items with tooltips or other floating content:
<Stepper>
<Tooltip>
<Stepper.Item>...</Stepper.Item>
</Tooltip>
</Stepper>| import type {StepperItemView} from './types'; | ||
| import {b} from './utils'; | ||
|
|
||
| import './Stepper.scss'; |
There was a problem hiding this comment.
You already imported this in Stepper.tsx
|
|
||
| import {b} from './utils'; | ||
|
|
||
| import './Stepper.scss'; |
There was a problem hiding this comment.
You already imported this in Stepper.tsx
| export const Size = { | ||
| render: (args) => { | ||
| return ( | ||
| <Flex direction="column" gap={4}> |
There was a problem hiding this comment.
Let's use Showcase component for that
There was a problem hiding this comment.
Are u mean to move this code to custom Showcase component?
| export const StepperSeparator = ({separator}: StepperSeparatorProps) => { | ||
| return ( | ||
| <div className={b('separator')} aria-hidden={true}> | ||
| {separator ?? <Icon data={ChevronRight} />} |
There was a problem hiding this comment.
We should use useDirection to support RTL and change the default icon to ChevronLeft
src/components/Stepper/Stepper.scss
Outdated
| flex-wrap: nowrap; | ||
| overflow-x: auto; |
There was a problem hiding this comment.
Why we need these styles? This cause focus ring to be clipped
bityutskiyAO
force-pushed
the
feat/create-stepper-component
branch
from
90cf55e to
bc8e38c
Compare
