Skip to content

Commit

Permalink
[wb1812.3.migratewb] Migrate Wonder Blocks off old id providers (#2391)
Browse files Browse the repository at this point in the history
## Summary:
This is the last piece in the first batch of work. This migrates all Wonder Blocks components off our old ID providers and onto the new `Id` component.

There are also some documentation tweaks to make the deprecation clearer in our stories, since that's some primary documentation for folks.

With this PR, we can cut a release and then begin updating consumer repos accordingly. First, to include this update, then to migrate them off the old ways and onto the new.

### Release process:
Once this small stack of PRs are landed and released, the following PRs need to be updated with that release, then landed/deployed:

- Perseus: Khan/perseus#2007
- Webapp
  - Khan/webapp#28105
  - Khan/webapp#28127


Issue: WB-1812

## Test plan:
`yarn test`
`yarn typecheck`
`yarn start:storybook`

Author: somewhatabstract

Reviewers: jandrade, somewhatabstract

Required Reviewers:

Approved By: jandrade

Checks: ⌛ Lint / Lint (ubuntu-latest, 20.x), ⌛ Check build sizes (ubuntu-latest, 20.x), ⌛ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 20.x), ⌛ Publish npm snapshot (ubuntu-latest, 20.x), ⌛ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ⏭️  dependabot

Pull Request URL: #2391
  • Loading branch information
somewhatabstract authored Dec 16, 2024
1 parent 897686b commit 56d961f
Show file tree
Hide file tree
Showing 34 changed files with 259 additions and 292 deletions.
14 changes: 14 additions & 0 deletions .changeset/witty-panthers-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
"@khanacademy/wonder-blocks-search-field": major
"@khanacademy/wonder-blocks-accordion": major
"@khanacademy/wonder-blocks-dropdown": major
"@khanacademy/wonder-blocks-popover": major
"@khanacademy/wonder-blocks-testing": major
"@khanacademy/wonder-blocks-tooltip": major
"@khanacademy/wonder-blocks-switch": major
"@khanacademy/wonder-blocks-modal": major
"@khanacademy/wonder-blocks-form": major
"@khanacademy/wonder-blocks-core": patch
---

- Migrate Wonder Blocks components off old id providers and onto new `Id` component
39 changes: 39 additions & 0 deletions __docs__/wonder-blocks-core/id.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import * as React from "react";
import {Meta, Story, Canvas} from "@storybook/blocks";
import * as IdStories from "./id.stories";

<Meta of={IdStories} />

# Id

`Id` is a component that provides an identifier to its children.

It is useful for situations where the `useId` hook cannot be easily used,
such as in class-based components.

If an `id` prop is provided, that is passed through to the children;
otherwise, a unique identifier is generated.

## Usage

```tsx
import {Id} from "@khanacademy/wonder-blocks-core";

<Id id={maybeId}>{(id) => <div id={id}>Hello, world!</div>}</Id>;
```

## Examples

### 1. Generating an id

An identifier will always be generated if an `id` prop is not provided, or the
provided `id` property is falsy.

<Canvas withSource="open" of={IdStories.GeneratedIdExample} />

### 2. Passthrough an id

If an `id` prop is provided and it is truthy, that value will be passed through
to the children.

<Canvas sourceState="shown" of={IdStories.PassedThroughIdExample} />
8 changes: 7 additions & 1 deletion __docs__/wonder-blocks-core/use-unique-id.mdx
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import {Meta, Story, Canvas} from "@storybook/blocks";
import * as UseUniqueIdStories from './use-unique-id.stories';
import * as UseUniqueIdStories from "./use-unique-id.stories";

<Meta of={UseUniqueIdStories} />

# `useUniqueIdWithoutMock`

DEPRECATED: Will be removed in a future release. Use `useId` from React or
the `Id` component.

This hook is similar to `<UniqueIDProvider mockOnFirstRender={false}>`.
It will return `null` on the initial render and then the same identifier
factory for each subsequent render. The identifier factory is unique to
Expand All @@ -19,6 +22,9 @@ render tree.

# `useUniqueIdWithMock`

DEPRECATED: Will be removed in a future release. Use `useId` from React or
the `Id` component.

This hook is similar to `<UniqueIDProvider mockOnFirstRender={true}>`.
It will return a mock identifier factory on the initial render that doesn'that
guarantee identifier uniqueness. Mock mode can help things appear on the screen
Expand Down
11 changes: 5 additions & 6 deletions __docs__/wonder-blocks-timing/with-action-scheduler.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-disable import/no-deprecated */
import * as React from "react";
import {Meta} from "@storybook/react";
import {IDProvider, View} from "@khanacademy/wonder-blocks-core";
import {Id, View} from "@khanacademy/wonder-blocks-core";

import {
Unmounter,
Expand Down Expand Up @@ -30,7 +29,7 @@ export default {
} as Meta;

export const IncorrectUsage = () => (
<IDProvider id="incorrect" scope="example">
<Id>
{(id) => (
<View>
<Unmounter>
Expand All @@ -39,11 +38,11 @@ export const IncorrectUsage = () => (
<View id={id} />
</View>
)}
</IDProvider>
</Id>
);

export const CorrectUsage = () => (
<IDProvider id="correct" scope="example">
<Id>
{(id) => (
<View>
<Unmounter>
Expand All @@ -52,5 +51,5 @@ export const CorrectUsage = () => (
<View id={id} />
</View>
)}
</IDProvider>
</Id>
);
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import * as React from "react";
import {StyleSheet} from "aphrodite";
import type {StyleDeclaration} from "aphrodite";

// eslint-disable-next-line import/no-deprecated
import {useUniqueIdWithMock, View} from "@khanacademy/wonder-blocks-core";
import {View} from "@khanacademy/wonder-blocks-core";
import * as tokens from "@khanacademy/wonder-blocks-tokens";
import {Body} from "@khanacademy/wonder-blocks-typography";
import type {AriaProps, StyleType} from "@khanacademy/wonder-blocks-core";

import {useId} from "react";
import type {AccordionCornerKindType} from "./accordion";
import AccordionSectionHeader from "./accordion-section-header";

Expand Down Expand Up @@ -204,15 +204,15 @@ const AccordionSection = React.forwardRef(function AccordionSection(

const controlledMode = expanded !== undefined && onToggle;

// eslint-disable-next-line import/no-deprecated
const ids = useUniqueIdWithMock();
const sectionId = id ?? ids.get("accordion-section");
const uniqueSectionId = useId();
const sectionId = id ?? uniqueSectionId;
// We need an ID for the header so that the content section's
// aria-labelledby attribute can point to it.
const headerId = id ? `${id}-header` : ids.get("accordion-section-header");
const uniqueHeaderId = useId();
const headerId = id ? `${id}-header` : uniqueHeaderId;
// We need an ID for the content section so that the opener's
// aria-controls attribute can point to it.
const sectionContentUniqueId = ids.get("accordion-section-content");
const sectionContentUniqueId = useId();

const sectionStyles = _generateStyles(
cornerKind,
Expand Down
6 changes: 3 additions & 3 deletions packages/wonder-blocks-core/src/components/id-provider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ type Props = {
};

/**
* @deprecated This component is deprecated and will be removed in an
* upcoming release. Migrate existing code to use `useId` or the `Id` component.
*
* This is a wrapper that returns an identifier. If the `id` prop is set, the
* component will return the same id to be consumed by its children. Otherwise,
* a unique id will be provided. This is beneficial for accessibility purposes,
Expand All @@ -54,9 +57,6 @@ type Props = {
* )}
* </IDProvider>
* ```
*
* @deprecated Use `useId` for your ID needs. If you are in a class-based
* component and cannot use hooks, then use the `Id` component.
*/
export default class IDProvider extends React.Component<Props> {
static defaultId = "wb-id";
Expand Down
5 changes: 4 additions & 1 deletion packages/wonder-blocks-core/src/components/id.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ type Props = {
};

/**
* A component that provides an identifier to its children.
* `Id` is a component that provides an identifier to its children.
*
* It is useful for situations where the `useId` hook cannot be easily used,
* such as in class-based components.
*
* If an `id` prop is provided, that is passed through to the children;
* otherwise, a unique identifier is generated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ type Props = {
};

/**
* @deprecated This component is deprecated and will be removed in an
* upcoming release. Migrate existing code to use `useId` or the `Id` component.
*
* The `UniqueIDProvider` component is how Wonder Blocks components obtain
* unique identifiers. This component ensures that server-side rendering and
* initial client rendering match while allowing the provision of unique
Expand All @@ -70,9 +73,6 @@ type Props = {
* )}
* </UniqueIDProvider>
* ```
*
* @deprecated Use `useId` for your ID needs. If you are in a class-based
* component and cannot use hooks, then use the `Id` component.
*/
export default class UniqueIDProvider extends React.Component<Props> {
// @ts-expect-error [FEI-5019] - TS2564 - Property '_idFactory' has no initializer and is not definitely assigned in the constructor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -907,11 +907,8 @@ describe("ActionMenu", () => {
const opener = await screen.findByRole("button");

// Assert
// Expect autogenerated id to be in the form uid-action-menu-opener-[number]-wb-id
expect(opener).toHaveAttribute(
"id",
expect.stringMatching(/^uid-action-menu-opener-\d+-wb-id$/),
);
// Expect autogenerated id
expect(opener).toHaveAttribute("id", expect.any(String));
});

it("Should use the `id` prop if provided", async () => {
Expand All @@ -929,6 +926,7 @@ describe("ActionMenu", () => {
// Assert
expect(opener).toHaveAttribute("id", id);
});

it("Should auto-generate an id for the dropdown if `dropdownId` prop is not provided", async () => {
// Arrange
render(
Expand All @@ -945,10 +943,7 @@ describe("ActionMenu", () => {
// Assert
expect(
await screen.findByRole("menu", {hidden: true}),
).toHaveAttribute(
"id",
expect.stringMatching(/^uid-action-menu-dropdown-\d+-wb-id$/),
);
).toHaveAttribute("id", expect.any(String));
});

it("Should use the `dropdownId` prop if provided", async () => {
Expand Down Expand Up @@ -1007,10 +1002,7 @@ describe("ActionMenu", () => {

// Assert
expect(opener).toHaveAttribute("aria-controls", dropdown.id);
expect(opener).toHaveAttribute(
"aria-controls",
expect.stringMatching(/^uid-action-menu-dropdown-\d+-wb-id$/),
);
expect(dropdown.id).toBeString();
});

it("Should set the `aria-controls` attribute on the custom opener to the provided dropdownId prop", async () => {
Expand All @@ -1035,7 +1027,7 @@ describe("ActionMenu", () => {

// Assert
expect(opener).toHaveAttribute("aria-controls", dropdown.id);
expect(opener).toHaveAttribute("aria-controls", dropdownId);
expect(dropdown.id).toBe(dropdownId);
});

it("Should set the `aria-controls` attribute on the custom opener to the auto-generated dropdownId", async () => {
Expand All @@ -1058,10 +1050,7 @@ describe("ActionMenu", () => {

// Assert
expect(opener).toHaveAttribute("aria-controls", dropdown.id);
expect(opener).toHaveAttribute(
"aria-controls",
expect.stringMatching(/^uid-action-menu-dropdown-\d+-wb-id$/),
);
expect(dropdown.id).toBeString();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1828,11 +1828,9 @@ describe("MultiSelect", () => {
const opener = await screen.findByRole("button");

// Assert
expect(opener).toHaveAttribute(
"id",
expect.stringMatching(/^uid-multi-select-opener-\d+-wb-id$/),
);
expect(opener).toHaveAttribute("id", expect.any(String));
});

it("Should use the `id` prop if provided", async () => {
// Arrange
const id = "test-id";
Expand All @@ -1849,6 +1847,7 @@ describe("MultiSelect", () => {
// Assert
expect(opener).toHaveAttribute("id", id);
});

it("Should auto-generate an id for the dropdown if `dropdownId` prop is not provided", async () => {
// Arrange
const {userEvent} = doRender(
Expand All @@ -1866,11 +1865,9 @@ describe("MultiSelect", () => {
// Assert
expect(
await screen.findByRole("listbox", {hidden: true}),
).toHaveAttribute(
"id",
expect.stringMatching(/^uid-multi-select-dropdown-\d+-wb-id$/),
);
).toHaveAttribute("id", expect.any(String));
});

it("Should use the `dropdownId` prop if provided", async () => {
// Arrange
const dropdownId = "test-id";
Expand Down Expand Up @@ -1930,10 +1927,7 @@ describe("MultiSelect", () => {

// Assert
expect(opener).toHaveAttribute("aria-controls", dropdown.id);
expect(opener).toHaveAttribute(
"aria-controls",
expect.stringMatching(/^uid-multi-select-dropdown-\d+-wb-id$/),
);
expect(opener).toHaveAttribute("aria-controls", expect.any(String));
});

it("Should set the `aria-controls` attribute on the custom opener to the provided dropdownId prop", async () => {
Expand Down Expand Up @@ -1983,10 +1977,7 @@ describe("MultiSelect", () => {

// Assert
expect(opener).toHaveAttribute("aria-controls", dropdown.id);
expect(opener).toHaveAttribute(
"aria-controls",
expect.stringMatching(/^uid-multi-select-dropdown-\d+-wb-id$/),
);
expect(opener).toHaveAttribute("aria-controls", expect.any(String));
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1396,11 +1396,9 @@ describe("SingleSelect", () => {
const opener = await screen.findByRole("button");

// Assert
expect(opener).toHaveAttribute(
"id",
expect.stringMatching(/^uid-single-select-opener-\d+-wb-id$/),
);
expect(opener).toHaveAttribute("id", expect.any(String));
});

it("Should use the `id` prop if provided", async () => {
// Arrange
const id = "test-id";
Expand All @@ -1417,6 +1415,7 @@ describe("SingleSelect", () => {
// Assert
expect(opener).toHaveAttribute("id", id);
});

it("Should auto-generate an id for the dropdown if `dropdownId` prop is not provided", async () => {
// Arrange
const {userEvent} = doRender(
Expand All @@ -1434,11 +1433,9 @@ describe("SingleSelect", () => {
// Assert
expect(
await screen.findByRole("listbox", {hidden: true}),
).toHaveAttribute(
"id",
expect.stringMatching(/^uid-single-select-dropdown-\d+-wb-id$/),
);
).toHaveAttribute("id", expect.any(String));
});

it("Should use the `dropdownId` prop if provided", async () => {
// Arrange
const dropdownId = "test-id";
Expand Down Expand Up @@ -1506,10 +1503,7 @@ describe("SingleSelect", () => {

// Assert
expect(opener).toHaveAttribute("aria-controls", dropdown.id);
expect(opener).toHaveAttribute(
"aria-controls",
expect.stringMatching(/^uid-single-select-dropdown-\d+-wb-id$/),
);
expect(opener).toHaveAttribute("aria-controls", expect.any(String));
});

it("Should set the `aria-controls` attribute on the custom opener to the provided dropdownId prop", async () => {
Expand Down Expand Up @@ -1561,10 +1555,7 @@ describe("SingleSelect", () => {

// Assert
expect(opener).toHaveAttribute("aria-controls", dropdown.id);
expect(opener).toHaveAttribute(
"aria-controls",
expect.stringMatching(/^uid-single-select-dropdown-\d+-wb-id$/),
);
expect(opener).toHaveAttribute("aria-controls", expect.any(String));
});
});

Expand Down
Loading

0 comments on commit 56d961f

Please sign in to comment.