feat(DATAGO-120289): Add stepper ui component #744
Conversation
ziyanwan
requested review from
DevSol-UQ,
JKaram,
bwiebe-solace and
lgh-solace
WhiteSource Policy Violation Summary✅︎ No Blocking Whitesource Policy Violations found in solaceai/solace-agent-mesh-ui-pr-744! |
| ...rest, | ||
| useStepper, | ||
| Stepper: { | ||
| Provider: ({ variant = "horizontal", labelOrientation = "horizontal", tracking = false, children, className, ...props }) => { |
There was a problem hiding this comment.
There's a lot going on in this file which makes it a little hard to follow. Notably, there's a lot of sub-components, and we're also mixing having some defined here vs. as top-level constants in the file. For maintainability, we should probably:
- Use a single pattern for these components on Stepper, if possible. My preference would be extracting these into separate constants to make the separation of responsibilities clear.
- Consider splitting-up this file to better divide the responsibilities (eg. creating a "stepper" directory with a few files). Maybe the hook(s) could be in one, the sub-components in another (or even individual files if we think they might grow in complexity over time), reusable types in one, etc. This will also have the benefit of fixing the Copilot-flagged fast refresh warnings.
There was a problem hiding this comment.
Thank you, Ben.
We are likely to switch to MRC components in the future. For now, I wouldn't spend too much time on the refactoring.
| @@ -0,0 +1,377 @@ | |||
| import { Slot } from "@radix-ui/react-slot"; | |||
| import * as Stepperize from "@stepperize/react"; | |||
There was a problem hiding this comment.
Consider scoping this import more narrowly to help with tree-shaking. If we're concerned about name collisions, we can do something like import { Stepper as StepperizeStepper } as needed.
There was a problem hiding this comment.
Good point, thank you, Ben.
@stepperize/react has 6 outputs, Get, Step, Stepper is from @stepperize/core, and then {type ScopedProps, type StepperReturn, defineStepper}. All of these 6 exports are used in this component. Tree-shaking won't help here.
|
One more thought: we have a stepper in the config portal UI (ie. when running |
| "@radix-ui/react-slot": "^1.2.3", | ||
| "@radix-ui/react-tabs": "^1.1.12", | ||
| "@radix-ui/react-tooltip": "^1.2.7", | ||
| "@stepperize/react": "^5.1.9", |
There was a problem hiding this comment.
Just curious, do you have any idea of the size of this new import?
There was a problem hiding this comment.
NPM says it has 7.95 kB unpacked.
As a comparison, the one above it, @radix-ui/react-tooltip has 135 kB, 17 times more.
So I wouldn't concern about the size.
| </nav> | ||
| ); | ||
| }, | ||
| Step: ({ children, className, icon, ...props }) => { |
There was a problem hiding this comment.
We could consider adding test ids here and there now to facilitate testing.
|
And perhaps a storybook test, if not now then later? |
|
What is the purpose of this change?
How was this change implemented?
How was this change tested?
Is there anything the reviewers should focus on/be aware of?
Demos
Screen.Recording.2026-01-06.at.12.30.45.PM.mov