feat(step-flow): add new component #6481

tomdavies73 · 2023-12-08T16:38:27Z

Proposed behaviour

This PR will add the StepFlow component which aims to help users advance through a user journey, and keep track of their progress. This is done initially with a very simple default configuration, with just the current title, current step and the total steps in the journey. This can also be contextualised further with an optional category prop.

However, a progress indicator can also be rendered via the showProgressIndicator prop, providing an added decorative indicator of progress, with indicators holding three individual states: completed, in progress, not completed ; each with their own background colours.

A close icon can also be rendered, with its own onDismiss prop, allowing consumers to pass functions to be called onClick such as closing a modal.

The component is also flexible, allowing for multiple steps to be set with the currentStep and totalSteps props, both can be between 1-8. Also currentStep can never exceed the value of totalSteps. When these are changed, the label underneath the title will change, as well as the indicator bar (if rendered).

The component also has built-in accessibility considerations with a focus ref being provided which provides consumers with the ability to pull focus to a visually hidden DOM element containing a formatted string with all of the relevant titles, category, step progress contained within.

This ensures screen reader users are given all the relevant information through semantic HTML on first navigation, and then when they progress through the journey, using buttons, for example focus is then pulled to the DOM element shown above. This will announce all of the relevant text content and then allow them to navigate down the page through all the relevant content below.

Current behaviour

Currently there is no accessible component which provides means to track and display a step journey or sequence.

Checklist

QA

Tested in CodeSandbox/storybook
Add new Cypress test coverage if required
Carbon implementation matches Design System/designs
UI Tests GitHub check reviewed if required

Additional context

Testing instructions

The following CodeSandbox is an example of the broken behaviour.
You can see the new behaviour by looking at the version in the comment by codesandbox[bot].

codesandbox-ci · 2023-12-08T16:41:09Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 810ecb2:

Sandbox	Source
carbon-quickstart	Configuration
carbon-quickstart-typescript	Configuration
carbon-quickstart	PR

edleeks87

Great effort @tomdavies73, especially on the considerations you've given to accessibility. I've left some comments but they're fairly straightforward to address, on the whole really good work 👏

edleeks87 · 2023-12-15T11:18:48Z

src/components/step-flow/step-flow.style.ts

+  flex-direction: column;
+`;
+
+const StyledCategoryText = styled(Typography)`


Comment: we can probably just use Typography here directly as it already supports the interface for all this styling

edleeks87 · 2023-12-15T11:19:41Z

src/components/step-flow/step-flow.style.ts

+
+const StyledTitleTextContainer = styled.span``;
+
+const StyledVisibleTitleText = styled.span`


Comment: if you simplify the HTML like Ive suggested above you can probably just pass these to Typography directly

edleeks87 · 2023-12-15T11:23:43Z

src/components/step-flow/step-flow.style.ts

+
+const StyledProgressIndicator = styled.span<StyledProgressIndicatorProps>`
+  background-color: ${({ isCompleted, isInProgress }) =>
+    calculateProgressIndicatorColor({ isCompleted, isInProgress })};


suggestion (non-blocking): you don't need to destructure here props are passed implicitly so you could just do:

background-color: ${calculateProgressIndicatorColor};

edleeks87 · 2023-12-15T11:25:46Z

src/components/step-flow/step-flow.style.ts

+  background-color: ${({ isCompleted, isInProgress }) =>
+    calculateProgressIndicatorColor({ isCompleted, isInProgress })};
+  width: 100%;
+  height: 8px;


comment: wherever possible we should use tokens such as --spacing150, --borderRadius100 etc.

Comment relates to whole file (font sizes etc)

edleeks87 · 2023-12-15T11:29:49Z

src/components/step-flow/step-flow.component.tsx

+  /** Set the variant of the internal 'Typography' component which contains the category.
+   * However, despite the chosen variant the styling will always be overridden.
+   */
+  categoryTypographyVariant?: VariantTypes;


Suggested change

categoryTypographyVariant?: VariantTypes;

categoryVariant? :Extract<VariantTypes, "h1" | "h2" | "h3" | "h4" | "h5" | "p">;

Naming is non-blocking for me but I think we should limit the elements here as some don't seem viable for this use case

edleeks87 · 2023-12-15T11:40:32Z

src/components/step-flow/step-flow.component.tsx

+  return (
+    <StyledStepFlow {...rest} {...tagComponent("step-flow", rest)}>
+      <StyledStepContent>
+        <StyledStepContentText>


Comment: this would render an empty container if no category is set. We could probably get away with using Typography directly here is we add support for aria-hidden etc

edleeks87 · 2023-12-15T11:44:12Z

src/components/step-flow/step-flow.component.tsx

+    );
+  });
+
+  const closeIcon = (


comment: lets add an aria-label here and support translations on it etc

edleeks87 · 2023-12-15T11:44:45Z

src/components/step-flow/step-flow.pw.tsx

+    await checkAccessibility(page);
+  });
+
+  test("should pass accessibility checks when 'showCloseIcon' is true", async ({


suggestion: maybe worth focusing the close icon and then checking against axe here

edleeks87 · 2023-12-15T11:45:59Z

src/components/step-flow/step-flow.spec.tsx

+      );
+
+      expect(
+        container.querySelector('[data-element="close"]')


comment: if we set and aria-label (see above) we can get by label here instead of using a container to query.

edleeks87 · 2023-12-15T11:46:49Z

src/components/step-flow/step-flow.stories.mdx

+
+<StyledSystemProps of={StepFlow} margin noHeader />
+
+<TranslationKeysTable


comment: don't forget to update this with the close aria label

DipperTheDan

Excellent work on this, @tomdavies73. Just a few comments from me, nothing serious though 😄👍🏻

DipperTheDan · 2024-01-09T14:36:24Z

src/components/step-flow/step-flow.pw.tsx

+
+const testData = [CHARACTERS.DIACRITICS, CHARACTERS.SPECIALCHARACTERS];
+
+test.describe("Prop checks for Portrait component", () => {


typo:

Suggested change

test.describe("Prop checks for Portrait component", () => {

test.describe("Prop checks for Step Flow component", () => {

Genuinely do not know how i missed this

DipperTheDan · 2024-01-09T14:38:59Z

src/components/step-flow/step-flow.pw.tsx

+      <StepFlowComponent currentStep={1} totalSteps={1} showProgressIndicator />
+    );
+
+    const backgroundColorToken = await getDesignTokensByCssProperty(


suggestion(non-blocking): You may not need this await here.

Suggested change

const backgroundColorToken = await getDesignTokensByCssProperty(

const backgroundColorToken = getDesignTokensByCssProperty(

I think we do need an await here as its a promise, without it backgroundColorToken returns undefined

DipperTheDan · 2024-01-09T14:39:23Z

src/components/step-flow/step-flow.pw.tsx

+      <StepFlowComponent currentStep={2} totalSteps={3} showProgressIndicator />
+    );
+
+    const backgroundColorToken = await getDesignTokensByCssProperty(


suggestion(non-blocking): Same again. You may not need this await here.

Suggested change

const backgroundColorToken = await getDesignTokensByCssProperty(

const backgroundColorToken = getDesignTokensByCssProperty(

I think we do need an await here as its a promise, without it backgroundColorToken returns undefined

DipperTheDan · 2024-01-09T14:39:46Z

src/components/step-flow/step-flow.pw.tsx

+      <StepFlowComponent currentStep={2} totalSteps={3} showProgressIndicator />
+    );
+
+    const backgroundColorToken = await getDesignTokensByCssProperty(


suggestion(non-blocking): Same again. You may not need this await here.

Suggested change

const backgroundColorToken = await getDesignTokensByCssProperty(

const backgroundColorToken = getDesignTokensByCssProperty(

I think we do need an await here as its a promise, without it backgroundColorToken returns undefined

DipperTheDan · 2024-01-09T14:40:33Z

src/components/step-flow/step-flow.pw.tsx

+    await stepFlowDismissIcon(page).click();
+    expect(callbackCount).toBe(1);


nitpick(non-blocking): space here otherwise the QA's will break your legs.

Suggested change

await stepFlowDismissIcon(page).click();

expect(callbackCount).toBe(1);

await stepFlowDismissIcon(page).click();

expect(callbackCount).toBe(1);

DipperTheDan · 2024-01-09T14:41:06Z

src/components/step-flow/step-flow.pw.tsx

+    await button(page).nth(1).click();
+    await expect(stepFlowTitleTextWrapper(page)).toBeFocused();


nitpick(non-blocking): Same again with the broken legs.

Suggested change

await button(page).nth(1).click();

await expect(stepFlowTitleTextWrapper(page)).toBeFocused();

await button(page).nth(1).click();

await expect(stepFlowTitleTextWrapper(page)).toBeFocused();

DipperTheDan · 2024-01-09T14:42:11Z

src/components/step-flow/step-flow.pw.tsx

+    await checkAccessibility(page);
+  });
+
+  test("should pass accessibility checks when component is rendered with a ref", async ({


suggestion(non-blocking): Just because the example is called StepFlowComponentWithRefAndButtons

Suggested change

test("should pass accessibility checks when component is rendered with a ref", async ({

test("should pass accessibility checks when component is rendered with a ref and buttons", async ({

edleeks87

Real good updates @tomdavies73, especially how thorough you've been in adding additional testing 👍

edleeks87 · 2024-01-09T16:36:01Z

src/components/step-flow/step-flow.spec.tsx

+
+    it("when the 'titleVariant' prop is not passed, the variant is h1 by default", () => {
+      const { container } = render(
+        <StepFlow title="this title is a h1" currentStep={5} totalSteps={6} />


comment (non-blocking, don't actually change anything): I suppose depending if you say aitch or haitch could affect whether it's a h1 or an h1 here 😆

edleeks87 · 2024-01-09T16:45:09Z

src/components/step-flow/step-flow.spec.tsx

+      const user = userEvent.setup();
+      const { container } = render(<MockComponent />);
+      const button = screen.getByRole("button");
+      button.focus();


comment: I don't think you need to focus the button here (just click) and given the what this is testing it may be a little confusing

edleeks87 · 2024-01-09T16:46:06Z

src/components/step-flow/step-flow.stories.mdx

@@ -39,9 +40,9 @@ import StepFlow from "carbon-react/lib/components/step-flow";

 ## Examples

-The `titleRef` prop is mandatory, and must be passed when the component is used to ensure focus is pulled to the components title whenever
+A `ref` is **strongly** recommended to be used, to ensure focus is programmatically moved to a a title div when


Suggestion (non-blocking):

To ensure focus is programmatically moved to the `title` of a given `StepFlow` when a user advances through each step, pass a `ref` is **strongly** recommended. This ensures screen reader users are made aware of all information throughout the user journey

harpalsingh · 2024-01-17T13:55:35Z

We (myself and @tempertemper )are happy with the suggested accessibility solution, however request from a semantic pov that the current h1/h2/h3 spec is changed to span/h1/span, and the ability to change the h1 semantically to h2 if required. Visually nothing will change from the design.

harpalsingh · 2024-01-22T14:45:27Z

As discussed with @tempertemper changes made so only the title is h1 by default but can be changed to a h2 and the Category and steps are now just spans.

creates a new carbon component to aid consumers in creating and offering means for end users to advance through a user journey. The component also has built-in accessibility considerations with a custom ref handle being provided which provides consumers with the ability to programmatically move focus to a visually hidden DOM element which consists of a formatted string with the relevant title, category, and step progress

carbonci · 2024-02-06T16:53:08Z

🎉 This PR is included in version 125.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

tomdavies73 added Pending Review Pending QA Pending UX QA labels Dec 8, 2023

tomdavies73 self-assigned this Dec 8, 2023

tomdavies73 requested review from a team as code owners December 8, 2023 16:38

tomdavies73 marked this pull request as draft December 8, 2023 16:38

tomdavies73 added the Pending a11y Review label Dec 8, 2023

edleeks87 self-requested a review December 12, 2023 15:41

edleeks87 reviewed Dec 15, 2023

View reviewed changes

tomdavies73 force-pushed the FE-6278 branch 3 times, most recently from c896c6d to dfa7725 Compare January 9, 2024 14:11

DipperTheDan reviewed Jan 9, 2024

View reviewed changes

edleeks87 reviewed Jan 9, 2024

View reviewed changes

tomdavies73 force-pushed the FE-6278 branch from a3029cb to 02f31f9 Compare January 10, 2024 15:24

edleeks87 previously approved these changes Jan 10, 2024

View reviewed changes

DipperTheDan previously approved these changes Jan 10, 2024

View reviewed changes

DipperTheDan marked this pull request as ready for review January 10, 2024 15:31

DipperTheDan added Passed Review Squad Front End and removed Pending Review labels Jan 10, 2024

harpalsingh added Passed UX QA and removed Pending UX QA labels Jan 16, 2024

tomdavies73 added Passed a11y Review and removed Pending a11y Review labels Jan 16, 2024

harpalsingh added Pending UX QA and removed Passed UX QA labels Jan 17, 2024

tomdavies73 dismissed stale reviews from DipperTheDan and edleeks87 via 57e4c8d January 19, 2024 11:26

tomdavies73 force-pushed the FE-6278 branch 3 times, most recently from 0ac1747 to 810ecb2 Compare January 19, 2024 12:10

harpalsingh added Passed UX QA and removed Pending UX QA labels Jan 22, 2024

stephenogorman previously approved these changes Feb 6, 2024

View reviewed changes

stephenogorman added Passed QA and removed Pending QA labels Feb 6, 2024

tomdavies73 dismissed stephenogorman’s stale review via 707ef4f February 6, 2024 16:33

tomdavies73 force-pushed the FE-6278 branch from 810ecb2 to 707ef4f Compare February 6, 2024 16:33

edleeks87 approved these changes Feb 6, 2024

View reviewed changes

stephenogorman approved these changes Feb 6, 2024

View reviewed changes

DipperTheDan approved these changes Feb 6, 2024

View reviewed changes

edleeks87 merged commit f25e78f into master Feb 6, 2024
22 checks passed

edleeks87 deleted the FE-6278 branch February 6, 2024 16:49

carbonci added the released label Feb 6, 2024

edleeks87 mentioned this pull request May 14, 2024

StepFlow - change title prop type #6732

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(step-flow): add new component #6481

feat(step-flow): add new component #6481

tomdavies73 commented Dec 8, 2023 •

edited

Loading

codesandbox-ci bot commented Dec 8, 2023 •

edited

Loading

edleeks87 left a comment

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

edleeks87 Dec 15, 2023

DipperTheDan left a comment

DipperTheDan Jan 9, 2024

tomdavies73 Jan 10, 2024

DipperTheDan Jan 9, 2024

tomdavies73 Jan 10, 2024

DipperTheDan Jan 9, 2024

tomdavies73 Jan 10, 2024

DipperTheDan Jan 9, 2024

tomdavies73 Jan 10, 2024

DipperTheDan Jan 9, 2024

DipperTheDan Jan 9, 2024

DipperTheDan Jan 9, 2024

edleeks87 left a comment

edleeks87 Jan 9, 2024

edleeks87 Jan 9, 2024

edleeks87 Jan 9, 2024

harpalsingh commented Jan 17, 2024

harpalsingh commented Jan 22, 2024

carbonci commented Feb 6, 2024


		const StyledTitleTextContainer = styled.span``;

		const StyledVisibleTitleText = styled.span`

	categoryTypographyVariant?: VariantTypes;
	categoryVariant? :Extract<VariantTypes, "h1" \| "h2" \| "h3" \| "h4" \| "h5" \| "p">;


		<StyledSystemProps of={StepFlow} margin noHeader />

		<TranslationKeysTable


		const testData = [CHARACTERS.DIACRITICS, CHARACTERS.SPECIALCHARACTERS];

		test.describe("Prop checks for Portrait component", () => {

	test.describe("Prop checks for Portrait component", () => {
	test.describe("Prop checks for Step Flow component", () => {

	const backgroundColorToken = await getDesignTokensByCssProperty(
	const backgroundColorToken = getDesignTokensByCssProperty(

		await stepFlowDismissIcon(page).click();
		expect(callbackCount).toBe(1);

		await button(page).nth(1).click();
		await expect(stepFlowTitleTextWrapper(page)).toBeFocused();

	test("should pass accessibility checks when component is rendered with a ref", async ({
	test("should pass accessibility checks when component is rendered with a ref and buttons", async ({

feat(step-flow): add new component #6481

feat(step-flow): add new component #6481

Conversation

tomdavies73 commented Dec 8, 2023 • edited Loading

Proposed behaviour

Current behaviour

Checklist

QA

Additional context

Testing instructions

codesandbox-ci bot commented Dec 8, 2023 • edited Loading

edleeks87 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

DipperTheDan 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

edleeks87 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

harpalsingh commented Jan 17, 2024

harpalsingh commented Jan 22, 2024

carbonci commented Feb 6, 2024

tomdavies73 commented Dec 8, 2023 •

edited

Loading

codesandbox-ci bot commented Dec 8, 2023 •

edited

Loading