Skip to content

Commit

Permalink
Migrate off deprecated ID APIs (#2014)
Browse files Browse the repository at this point in the history
## Summary:
This migrates Perseus off the deprecated ID APIs and onto the `useId`-based approach.

The lint rules remain in place until Wonder Blocks is released without the old APIs and Perseus has been updated to that version of Wonder Blocks. This won't happen until webapp has been migrated off the old APIs.

Issue: WB-1812

## Test plan:
`yarn typecheck`
`yarn lint`
`yarn test`

Author: somewhatabstract

Reviewers: mark-fitzgerald, catandthemachines

Required Reviewers:

Approved By: mark-fitzgerald, catandthemachines

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x)

Pull Request URL: #2014
  • Loading branch information
somewhatabstract authored Dec 16, 2024
1 parent 7a4b584 commit 763d2d0
Show file tree
Hide file tree
Showing 13 changed files with 61 additions and 81 deletions.
7 changes: 7 additions & 0 deletions .changeset/rare-bobcats-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@khanacademy/perseus-dev-ui": patch
"@khanacademy/perseus": patch
"@khanacademy/perseus-editor": patch
---

Migrate off deprecated ID generation APIs
15 changes: 6 additions & 9 deletions dev/gallery.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/* eslint monorepo/no-internal-import: "off", monorepo/no-relative-import: "off", import/no-relative-packages: "off" */
import Button from "@khanacademy/wonder-blocks-button";
import {useUniqueIdWithMock, View} from "@khanacademy/wonder-blocks-core";
import {View} from "@khanacademy/wonder-blocks-core";
import {OptionItem, MultiSelect} from "@khanacademy/wonder-blocks-dropdown";
import {Strut} from "@khanacademy/wonder-blocks-layout";
import SearchField from "@khanacademy/wonder-blocks-search-field";
import Switch from "@khanacademy/wonder-blocks-switch";
import {spacing} from "@khanacademy/wonder-blocks-tokens";
import {css, StyleSheet} from "aphrodite";
import * as React from "react";
import {useEffect, useMemo, useState} from "react";
import {useEffect, useId, useMemo, useState} from "react";

import {Renderer} from "../packages/perseus/src";
import {mockStrings} from "../packages/perseus/src/strings";
Expand Down Expand Up @@ -98,9 +98,6 @@ function capitalize(key: string): string {
}

export function Gallery() {
// TODO(WB-1812, somewhatabstract): Migrate to Id or useId
// eslint-disable-next-line no-restricted-syntax
const ids = useUniqueIdWithMock();
const params = useMemo(
() => new URLSearchParams(window.location.search),
[],
Expand Down Expand Up @@ -150,10 +147,10 @@ export function Gallery() {
return acc;
}, {});

const mobileId = ids.get("mobile");
const tooltipId = ids.get("tooltip");
const flagsId = ids.get("flags");
const searchId = ids.get("search");
const mobileId = useId();
const tooltipId = useId();
const flagsId = useId();
const searchId = useId();

const insertShowTooltips = ([question, i]): [PerseusRenderer, number] => {
Object.keys(question.widgets).forEach((widgetName) => {
Expand Down
7 changes: 2 additions & 5 deletions packages/perseus-editor/src/components/widget-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import {
iconChevronDown,
iconTrash,
} from "@khanacademy/perseus";
import {useUniqueIdWithMock} from "@khanacademy/wonder-blocks-core";
import {Strut} from "@khanacademy/wonder-blocks-layout";
import Switch from "@khanacademy/wonder-blocks-switch";
import {spacing} from "@khanacademy/wonder-blocks-tokens";
import * as React from "react";
import {useId} from "react";
import _ from "underscore";

import {iconChevronRight} from "../styles/icon-paths";
Expand Down Expand Up @@ -236,10 +236,7 @@ function LabeledSwitch(props: {
onChange: (value: boolean) => unknown;
}) {
const {label, ...switchProps} = props;
// TODO(WB-1812, somewhatabstract): Migrate to Id or useId
// eslint-disable-next-line no-restricted-syntax
const ids = useUniqueIdWithMock();
const id = ids.get("switch");
const id = useId();
return (
<>
<label htmlFor={id}>{label}</label>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import {View, useUniqueIdWithMock} from "@khanacademy/wonder-blocks-core";
import {View} from "@khanacademy/wonder-blocks-core";
import {Strut} from "@khanacademy/wonder-blocks-layout";
import Switch from "@khanacademy/wonder-blocks-switch";
import {spacing} from "@khanacademy/wonder-blocks-tokens";
import {LabelMedium} from "@khanacademy/wonder-blocks-typography";
import {StyleSheet} from "aphrodite";
import * as React from "react";
import {useId} from "react";

import type {StyleType} from "@khanacademy/wonder-blocks-core";

Expand All @@ -18,11 +19,7 @@ type Props = {
const LabeledSwitch = (props: Props) => {
const {checked, label, style, onChange} = props;

// TODO(WB-1812, somewhatabstract): Migrate to Id or useId
// eslint-disable-next-line no-restricted-syntax
const ids = useUniqueIdWithMock();
const switchId = ids.get("switch");

const switchId = useId();
return (
<View style={[styles.row, style]}>
<Switch id={switchId} checked={checked} onChange={onChange} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
* the dropdown for adding figures as well as the settings for each figure.
*/
import Button from "@khanacademy/wonder-blocks-button";
import {View, useUniqueIdWithMock} from "@khanacademy/wonder-blocks-core";
import {View} from "@khanacademy/wonder-blocks-core";
import {Strut} from "@khanacademy/wonder-blocks-layout";
import {spacing} from "@khanacademy/wonder-blocks-tokens";
import {StyleSheet} from "aphrodite";
import * as React from "react";
import {useId} from "react";

import Heading from "../../../components/heading";

Expand Down Expand Up @@ -43,9 +44,7 @@ const LockedFiguresSection = (props: Props) => {

const [isExpanded, setIsExpanded] = React.useState(true);

// TODO(WB-1812, somewhatabstract): Migrate to Id or useId
// eslint-disable-next-line no-restricted-syntax
const uniqueId = useUniqueIdWithMock().get("locked-figures-section");
const uniqueId = useId();
const {figures, onChange} = props;

function addLockedFigure(newFigure: LockedFigureType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ exports[`renderer snapshots correct answer: correct answer 1`] = `
>
<label
class="text_f1191h-o_O-LabelLarge_5s82ln"
for="uid-dropdown-widget-1-dropdown"
for=":r3:"
>
Test visible label
</label>
Expand All @@ -365,14 +365,14 @@ exports[`renderer snapshots correct answer: correct answer 1`] = `
data-testid="dropdown-live-region"
/>
<button
aria-controls=":r2:"
aria-controls=":r4:"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="listbox"
aria-invalid="false"
aria-label="Test ARIA label"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-dropdown-widget-1-dropdown"
id=":r3:"
role="combobox"
type="button"
>
Expand Down Expand Up @@ -429,7 +429,7 @@ exports[`renderer snapshots incorrect answer: incorrect answer 1`] = `
>
<label
class="text_f1191h-o_O-LabelLarge_5s82ln"
for="uid-dropdown-widget-2-dropdown"
for=":r6:"
>
Test visible label
</label>
Expand All @@ -444,14 +444,14 @@ exports[`renderer snapshots incorrect answer: incorrect answer 1`] = `
data-testid="dropdown-live-region"
/>
<button
aria-controls=":r4:"
aria-controls=":r7:"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="listbox"
aria-invalid="false"
aria-label="Test ARIA label"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-dropdown-widget-2-dropdown"
id=":r6:"
role="combobox"
type="button"
>
Expand Down Expand Up @@ -508,7 +508,7 @@ exports[`renderer snapshots initial render: initial render 1`] = `
>
<label
class="text_f1191h-o_O-LabelLarge_5s82ln"
for="uid-dropdown-widget-0-dropdown"
for=":r0:"
>
Test visible label
</label>
Expand All @@ -523,14 +523,14 @@ exports[`renderer snapshots initial render: initial render 1`] = `
data-testid="dropdown-live-region"
/>
<button
aria-controls=":r0:"
aria-controls=":r1:"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="listbox"
aria-invalid="false"
aria-label="Test ARIA label"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-dropdown-widget-0-dropdown"
id=":r0:"
role="combobox"
type="button"
>
Expand Down
11 changes: 4 additions & 7 deletions packages/perseus/src/util/react-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,10 @@ export default function render(
container: HTMLElement,
) {
/**
* `RenderStateRoot` is responsible for tracking whether it's
* the initial render or subsequent renders. It's used by
* `useUniqueId` and will be used by `UniqueIDProvider` and
* `WithSSRPlaceholder` in the future. We're placing it as
* high up in the render tree as possible to ensure that any
* components using that hook or those components will function
* correctly.
* `RenderStateRoot` is responsible for tracking whether it's the initial
* render or subsequent renders. It's used by `WithSSRPlaceholder`. We're
* placing it as high up in the render tree as possible to ensure that any
* components using that hook or those components will function correctly.
*/
const children = (
<RenderStateRoot throwIfNested={false}>{element}</RenderStateRoot>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ exports[`Dropdown widget should snapshot when opened: dropdown open 1`] = `
3 items
</span>
<button
aria-controls=":r2:"
aria-controls=":r4:"
aria-disabled="false"
aria-expanded="true"
aria-haspopup="listbox"
aria-invalid="false"
aria-label="Select an answer"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-dropdown-widget-1-dropdown"
id=":r3:"
role="combobox"
type="button"
>
Expand Down Expand Up @@ -105,14 +105,14 @@ exports[`Dropdown widget should snapshot: initial render 1`] = `
data-testid="dropdown-live-region"
/>
<button
aria-controls=":r0:"
aria-controls=":r1:"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="listbox"
aria-invalid="false"
aria-label="Select an answer"
class="button_vr44p2-o_O-shared_u51dsh-o_O-default_3ie67y"
id="uid-dropdown-widget-0-dropdown"
id=":r0:"
role="combobox"
type="button"
>
Expand Down
17 changes: 6 additions & 11 deletions packages/perseus/src/widgets/dropdown/dropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {UniqueIDProvider, View} from "@khanacademy/wonder-blocks-core";
import {Id, View} from "@khanacademy/wonder-blocks-core";
import {SingleSelect, OptionItem} from "@khanacademy/wonder-blocks-dropdown";
import {LabelLarge} from "@khanacademy/wonder-blocks-typography";
import * as React from "react";
Expand Down Expand Up @@ -86,10 +86,8 @@ class Dropdown extends React.Component<Props> implements Widget {
];

return (
// TODO(WB-1812, somewhatabstract): Migrate to Id or useId
// eslint-disable-next-line no-restricted-syntax
<UniqueIDProvider scope="dropdown-widget" mockOnFirstRender={true}>
{(ids) => (
<Id>
{(dropdownId) => (
<View
// NOTE(jared): These are required to prevent weird behavior
// When there's a dropdown in a zoomable table.
Expand All @@ -101,15 +99,12 @@ class Dropdown extends React.Component<Props> implements Widget {
}}
>
{this.props.visibleLabel && (
<LabelLarge
tag="label"
htmlFor={ids.get("dropdown")}
>
<LabelLarge tag="label" htmlFor={dropdownId}>
{this.props.visibleLabel}
</LabelLarge>
)}
<SingleSelect
id={ids.get("dropdown")}
id={dropdownId}
placeholder=""
onChange={(value) =>
this._handleChange(parseInt(value))
Expand All @@ -131,7 +126,7 @@ class Dropdown extends React.Component<Props> implements Widget {
</SingleSelect>
</View>
)}
</UniqueIDProvider>
</Id>
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ exports[`Explanation should snapshot when expanded: expanded 1`] = `
class="perseus-widget-container widget-nohighlight widget-inline"
>
<button
aria-controls="uid-explanation-widget-1-content"
aria-controls=":r1:"
aria-disabled="false"
aria-expanded="true"
class="button_vr44p2-o_O-shared_lwskrm-o_O-default_1hl5pu8-o_O-small_14crccx-o_O-inlineStyles_1s8anjv"
Expand All @@ -43,7 +43,7 @@ exports[`Explanation should snapshot when expanded: expanded 1`] = `
aria-hidden="false"
class="default_xu2jcg-o_O-content_1pprw9s-o_O-contentExpanded_1ojyq0m"
data-testid="content-container"
id="uid-explanation-widget-1-content"
id=":r1:"
>
<div
class="default_xu2jcg-o_O-contentWrapper_y9ev9r"
Expand Down Expand Up @@ -91,7 +91,7 @@ exports[`Explanation should snapshot: initial render 1`] = `
class="perseus-widget-container widget-nohighlight widget-inline"
>
<button
aria-controls="uid-explanation-widget-0-content"
aria-controls=":r0:"
aria-disabled="false"
aria-expanded="false"
class="button_vr44p2-o_O-shared_lwskrm-o_O-default_1hl5pu8-o_O-small_14crccx-o_O-inlineStyles_1s8anjv"
Expand All @@ -116,7 +116,7 @@ exports[`Explanation should snapshot: initial render 1`] = `
aria-hidden="true"
class="default_xu2jcg-o_O-content_1pprw9s-o_O-contentCollapsed_j2403j"
data-testid="content-container"
id="uid-explanation-widget-0-content"
id=":r0:"
>
<div
class="default_xu2jcg-o_O-contentWrapper_y9ev9r"
Expand Down
17 changes: 6 additions & 11 deletions packages/perseus/src/widgets/explanation/explanation.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {linterContextDefault} from "@khanacademy/perseus-linter";
import Button from "@khanacademy/wonder-blocks-button";
import {UniqueIDProvider, View} from "@khanacademy/wonder-blocks-core";
import {Id, View} from "@khanacademy/wonder-blocks-core";
import caretDown from "@phosphor-icons/core/regular/caret-down.svg";
import caretUp from "@phosphor-icons/core/regular/caret-up.svg";
import {StyleSheet} from "aphrodite";
Expand Down Expand Up @@ -128,17 +128,12 @@ class Explanation extends React.Component<Props, State> implements Widget {
];

return (
// TODO(WB-1812, somewhatabstract): Migrate to Id or useId
// eslint-disable-next-line no-restricted-syntax
<UniqueIDProvider
mockOnFirstRender={true}
scope="explanation-widget"
>
{(ids) => (
<Id>
{(contentId) => (
<>
<Button
aria-expanded={this.state.expanded}
aria-controls={ids.get("content")}
aria-controls={contentId}
endIcon={caretIcon}
kind="tertiary"
labelStyle={labelStyle}
Expand All @@ -150,7 +145,7 @@ class Explanation extends React.Component<Props, State> implements Widget {
</Button>

<View
id={ids.get("content")}
id={contentId}
style={contentStyling}
aria-hidden={!this.state.expanded}
testId="content-container"
Expand All @@ -167,7 +162,7 @@ class Explanation extends React.Component<Props, State> implements Widget {
</View>
</>
)}
</UniqueIDProvider>
</Id>
);
}
}
Expand Down
Loading

0 comments on commit 763d2d0

Please sign in to comment.