feat(component): add Stepper component #503

Spawbreaker · 2021-02-03T22:42:24Z

What

Adds the Stepper component, a new component to track progress on multi-step tasks.

Why

bc-prestonhuth · 2021-02-22T18:57:35Z

@chanceaclark Can you take a look at this when you have some time please? @Spawbreaker can you merge master into this branch so it's not out of date? Thanks!

chanceaclark · 2021-02-23T01:29:20Z

Hey @Spawbreaker @bc-prestonhuth, sorry about the wait. The last few weeks have been quite busy/blizzardous. I'll get around to reviewing this in the next day or so.

chanceaclark

Hey @Spawbreaker, I have a ton of comments regarding internal structure of this component and how to simplify it. To make this a little easier to communicate from my end, I created another branch to show the changes I would like to make:

Spawbreaker/big-design@stepper...chanceaclark:stepper-updates

I will also inline comments in regards to those changes for what I changed. Keep in mind if I comment on something, it may be the same for a bunch of places.

Otherwise it's looking really good! Love some of the CSS stuff you did (though I change some stuff 😉 )

chanceaclark · 2021-02-24T17:11:03Z

packages/big-design/src/components/Stepper/Stepper.tsx

+  active?: boolean;
+}
+
+export const Step: React.FC<StepProps> = ({ number, text, state, isLast }) => {


Move this component into its own file: .../src/components/Stepper/Step/Step.tsx

chanceaclark · 2021-02-24T17:14:18Z

packages/big-design/src/components/Stepper/Stepper.tsx

+  const [isCurrent, setIsCurrent] = useState(state === 'current');
+  const [isDone, setIsDone] = useState(state === 'done');
+  useEffect(() => {
+    setIsCurrent(state === 'current');
+    setIsDone(state === 'done');
+  }, [state]);


I would say that this Stepper component is not a stateful component. We should avoid having internal state where it's not needed. This is more of a component where you pass state into it and you can derive values from it. See what I mean here.

chanceaclark · 2021-02-24T17:16:21Z

packages/big-design/src/components/Stepper/styled.tsx

+export const StyledStepper = styled.div`
+  display: grid;


We should be extending from our built in component rather than defining our own display properties, where possible.

Suggested change

export const StyledStepper = styled.div`

display: grid;

export const StyledStepper = styled(Grid)`

That way you can use built in helpers:

<StyledStepper marginBottom="xLarge" />

chanceaclark · 2021-02-24T17:16:59Z

packages/big-design/src/components/Stepper/styled.tsx

+export const StyledStepper = styled.div`
+  display: grid;
+  margin-bottom: 1.5rem;
+  text-align: left;


Not needed.

Suggested change

text-align: left;

chanceaclark · 2021-02-24T17:33:18Z

packages/big-design/src/components/Stepper/Stepper.tsx

+export interface LightProps {
+  active?: boolean;
+}
+export interface DashProps {
+  active?: boolean;
+}
+export interface TextProps {
+  active?: boolean;
+}


If they have the same props, might want to name the interface something a bit more generic to reuse. In my branch I didn't use these, in favor of using dynamic props by extending our utility components, like so.

chanceaclark · 2021-02-24T17:42:47Z

packages/big-design/src/components/Stepper/Stepper.tsx

+  }, [state]);
+
+  return (
+    <StyledStep>


Couple of things with this component:

I think using a Flex is better that using a grid. According to the designs, each step should be the same length as the others and using that grid minmax(6rem, 14.25rem) logic will cause them to collapse non-uniformly on smaller screen sizes.

This is a big miss – we need to add in a11y props. I added them here. Here is also the https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_progressbar_role

chanceaclark · 2021-02-24T17:43:19Z

packages/big-design/src/components/Stepper/Stepper.tsx

+    <StyledStep>
+      <StyledLight active={isCurrent || isDone}>
+        {isDone ? (
+          <CheckIcon color="white" size="large" style={{ padding: '2px' }} />


We should almost never use the style attribute.

chanceaclark · 2021-02-24T17:45:53Z

packages/big-design/src/components/Stepper/styled.tsx

+export const StyledStep = styled.div`
+  display: grid;
+  ${({ theme }) => theme.breakpoints.tablet} {
+    grid-template-columns: 1.5rem 1fr;


When working with spacing value, use the ${({ theme }) => theme.spacing.<spacing value>} instead of hard coding rem units. If any values are bigger than our predefined spacing values, consider using ${({ theme }) => theme.spacing.remCalc(96)} // 6rem.

chanceaclark · 2021-02-24T17:48:31Z

packages/docs/pages/Stepper/StepperPage.tsx

+        const steps = ['Step 1', 'Step 2', 'Step 3'];
+        const currentStep = 0;
+
+        return <Stepper steps={steps} currentStep={currentStep} />;


Let's add some interaction to this example to show the different states. Suggestion:

Suggested change

return <Stepper steps={steps} currentStep={currentStep} />;

return (

<>

<Stepper steps={steps} currentStep={currentStep} />

<Button onClick={() => setCurrentStep(currentStep + 1)} disabled={currentStep === steps.length - 1}>

Next

</Button>

<Button onClick={() => setCurrentStep(0)} disabled={currentStep === 0}>

Reset

</Button>

</>

);

packages/big-design/src/components/Stepper/Stepper.tsx

chanceaclark · 2021-02-26T16:58:22Z

packages/big-design/src/components/Stepper/spec.tsx

+  const { container } = render(<Stepper steps={steps} currentStep={1} />);
+  expect(container.firstChild).toMatchSnapshot();
+});
+


Let's add some more tests to make sure passed in state works as expected (hint: no snapshot testing for these).

chanceaclark · 2021-02-26T17:06:40Z

packages/big-design/src/components/Stepper/Stepper.tsx

+  currentStep: number;
+}
+
+export const Stepper: React.FC<StepperProps> = memo(({ className, style, steps, currentStep, ...props }) => (


Instead of providing defaultProps below, we may want to inline them here. Maybe even exclude steps having a default prop. Also, we may want to check if the provided steps array has length to ensure we don't render anything if an empty array is provided. The main reasoning for all of this is to guard rendering in non-Typescript projects + empty arrays.

Suggested change

export const Stepper: React.FC<StepperProps> = memo(({ className, style, steps, currentStep, ...props }) => (

export const Stepper: React.FC<StepperProps> = memo(({ className, style, steps, currentStep = 0, ...props }) => steps.length ? (

// make sure to finish the ternary

chanceaclark · 2021-02-26T17:07:10Z

packages/docs/PropTables/StepperPropTable.tsx

+  },
+  {
+    name: 'currentStep',
+    types: 'number',


This should also be required.

chanceaclark · 2021-02-26T17:10:27Z

packages/docs/pages/Stepper/StepperPage.tsx

+    <H0>Stepper</H0>
+
+    <Text>
+      The <Code primary>Stepper</Code> component is used to display a set number of steps. Useful for guiding an user


Suggested change

The <Code primary>Stepper</Code> component is used to display a set number of steps. Useful for guiding an user

The <Code primary>Stepper</Code> component is used to display a set number of steps. Useful for guiding a user

chanceaclark · 2021-02-26T17:13:02Z

packages/docs/pages/Stepper/StepperPage.tsx

+    <CodePreview>
+      {/* jsx-to-string:start */}
+      {function Example() {
+        const steps = ['Step 1', 'Step 2', 'Step 3'];


🍹 Can we make these steps a little more "real world"? e.g. "Login", "Import products", etc...

chanceaclark

Looks good to me!

Spawbreaker requested a review from a team as a code owner February 3, 2021 22:42

Spawbreaker force-pushed the stepper branch from a7c5f1d to 4e494e9 Compare February 3, 2021 22:51

Spawbreaker force-pushed the stepper branch from 4e494e9 to fa5a4c8 Compare February 22, 2021 19:33

chanceaclark reviewed Feb 24, 2021

View reviewed changes

Spawbreaker force-pushed the stepper branch from fa5a4c8 to ee08d97 Compare February 25, 2021 23:29

chanceaclark self-requested a review February 26, 2021 16:33

chanceaclark reviewed Feb 26, 2021

View reviewed changes

feat(component): add Stepper component

fc708fa

Spawbreaker force-pushed the stepper branch from ee08d97 to fc708fa Compare March 1, 2021 19:00

chanceaclark approved these changes Mar 2, 2021

View reviewed changes

chanceaclark merged commit 994943c into bigcommerce:master Mar 8, 2021

chanceaclark mentioned this pull request Mar 9, 2021

Feature Request: Step Component #420

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(component): add Stepper component #503

feat(component): add Stepper component #503

Spawbreaker commented Feb 3, 2021

bc-prestonhuth commented Feb 22, 2021

chanceaclark commented Feb 23, 2021

chanceaclark left a comment

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 24, 2021

chanceaclark Feb 26, 2021

chanceaclark Feb 26, 2021

chanceaclark Feb 26, 2021

chanceaclark Feb 26, 2021

chanceaclark Feb 26, 2021

chanceaclark left a comment

	export const StyledStepper = styled.div`
	display: grid;
	export const StyledStepper = styled(Grid)`

-        return <Stepper steps={steps} currentStep={currentStep} />;
+        return (
+          <>
+            <Stepper steps={steps} currentStep={currentStep} />
+            <Button onClick={() => setCurrentStep(currentStep + 1)} disabled={currentStep === steps.length - 1}>
+              Next
+            </Button>
+            <Button onClick={() => setCurrentStep(0)} disabled={currentStep === 0}>
+              Reset
+            </Button>
+          </>
+        );

	export const Stepper: React.FC<StepperProps> = memo(({ className, style, steps, currentStep, ...props }) => (
	export const Stepper: React.FC<StepperProps> = memo(({ className, style, steps, currentStep = 0, ...props }) => steps.length ? (
	// make sure to finish the ternary

	The <Code primary>Stepper</Code> component is used to display a set number of steps. Useful for guiding an user
	The <Code primary>Stepper</Code> component is used to display a set number of steps. Useful for guiding a user

feat(component): add Stepper component #503

feat(component): add Stepper component #503

Conversation

Spawbreaker commented Feb 3, 2021

What

Why

bc-prestonhuth commented Feb 22, 2021

chanceaclark commented Feb 23, 2021

chanceaclark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chanceaclark left a comment

Choose a reason for hiding this comment