Skip to content

Commit

Permalink
[wb1670.1.react18] Upgrade to React 18 (#2380)
Browse files Browse the repository at this point in the history
## Summary:
This builds off @jandrade's second attempt (that was PR #2372). The first commit is @jandrade's work from that PR, the subsequent commits are my additional changes to get us over the finish line.

Issue: WB-1670

### Dropdown fixes
The `SingleSelect` virtualization was having an issue initially. The focus was not being shown on the first item on the list as it should. In investigation, I discovered a related issue where in virtualized lists, the first and last item could get skipped over when navigating with the keyboard. The following videos show the behavior for virtualized and non-virtualized variants to show that the issue has been fixed without breaking non-virtualized use.

This video shows the before behavior, with the missing focus (a bug introduced with the React 18 update) and the skipped focus during navigation (a long term bug).

https://github.com/user-attachments/assets/e7d3308e-33ed-4ed3-a9e7-5462404ae69c

This video shows things after fixing them as part of this PR.

https://github.com/user-attachments/assets/766dfb46-3358-474e-8718-e819e97c35e2

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

Author: somewhatabstract

Reviewers: jandrade, beaesguerra, somewhatabstract

Required Reviewers:

Approved By: jandrade, beaesguerra

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ 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), ⏭️  Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️  dependabot

Pull Request URL: #2380
  • Loading branch information
somewhatabstract authored Dec 6, 2024
1 parent d67761d commit e6abdd1
Show file tree
Hide file tree
Showing 84 changed files with 2,648 additions and 907 deletions.
38 changes: 38 additions & 0 deletions .changeset/five-pens-accept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
"@khanacademy/wonder-blocks-birthday-picker": major
"@khanacademy/wonder-blocks-testing-core": major
"@khanacademy/wonder-blocks-accordion": major
"@khanacademy/wonder-blocks-dropdown": major
"@khanacademy/wonder-blocks-popover": major
"@khanacademy/wonder-blocks-testing": major
"@khanacademy/wonder-blocks-theming": major
"@khanacademy/wonder-blocks-tooltip": major
"@khanacademy/wonder-blocks-timing": major
"@khanacademy/wonder-blocks-core": major
"@khanacademy/wonder-blocks-data": major
"@khanacademy/wonder-blocks-form": major
"@khanacademy/wonder-blocks-i18n": major
"@khanacademy/wb-dev-build-settings": major
"@khanacademy/wonder-blocks-banner": major
"@khanacademy/wonder-blocks-breadcrumbs": major
"@khanacademy/wonder-blocks-button": major
"@khanacademy/wonder-blocks-cell": major
"@khanacademy/wonder-blocks-clickable": major
"@khanacademy/wonder-blocks-grid": major
"@khanacademy/wonder-blocks-icon": major
"@khanacademy/wonder-blocks-icon-button": major
"@khanacademy/wonder-blocks-labeled-field": major
"@khanacademy/wonder-blocks-layout": major
"@khanacademy/wonder-blocks-link": major
"@khanacademy/wonder-blocks-modal": major
"@khanacademy/wonder-blocks-pill": major
"@khanacademy/wonder-blocks-progress-spinner": major
"@khanacademy/wonder-blocks-search-field": major
"@khanacademy/wonder-blocks-switch": major
"@khanacademy/wonder-blocks-tokens": major
"@khanacademy/wonder-blocks-toolbar": major
"@khanacademy/wonder-blocks-typography": major
"@khanacademy/wb-codemod": major
---

Upgrade to React 18
6 changes: 6 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ module.exports = {
"no-undef": "off",
},
},
{
files: ["**/*.stories.tsx"],
rules: {
"testing-library/no-await-sync-events": "off",
},
},
],
globals: {
// `no-undef` doesn't support `globalThis`, for details see
Expand Down
3 changes: 2 additions & 1 deletion .storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const parameters = {
},
};

export const decorators = [
const decorators = [
(Story, context) => {
const theme = context.globals.theme;
const enableRenderStateRootDecorator =
Expand All @@ -120,6 +120,7 @@ export const decorators = [

const preview: Preview = {
parameters,
decorators,
globalTypes: {
// Allow the user to select a theme from the toolbar.
theme: {
Expand Down
31 changes: 17 additions & 14 deletions __docs__/wonder-blocks-button/button.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from "react";
import {StyleSheet} from "aphrodite";
import type {Meta, StoryObj} from "@storybook/react";
import {expect, fireEvent, userEvent, within} from "@storybook/test";
import {expect, userEvent, within} from "@storybook/test";

import {MemoryRouter, Route, Switch} from "react-router-dom";

Expand Down Expand Up @@ -103,7 +103,6 @@ export const Tertiary: StoryComponentType = {

// Get HTML elements
const button = canvas.getByRole("button");
const computedStyleButton = getComputedStyle(button);
const innerLabel = canvas.getByTestId("test-button-inner-label");
const computedStyleLabel = getComputedStyle(innerLabel, ":after");

Expand All @@ -116,19 +115,23 @@ export const Tertiary: StoryComponentType = {
await expect(computedStyleLabel.height).toBe("2px");
await expect(computedStyleLabel.color).toBe("rgb(24, 101, 242)");

// TODO(WB-1808, somewhatabstract): This isn't working. I got it passing
// locally by calling `button.focus()` as well, but it was super flaky
// and never passed first time.
// Focus style
await fireEvent.focus(button);
await expect(computedStyleButton.outlineColor).toBe(
"rgb(24, 101, 242)",
);
await expect(computedStyleButton.outlineWidth).toBe("2px");

// Active (mouse down) style
// eslint-disable-next-line testing-library/prefer-user-event
await fireEvent.mouseDown(button);
await expect(innerLabel).toHaveStyle("color: rgb(27, 80, 179)");
await expect(computedStyleLabel.height).toBe("2px");
await expect(computedStyleLabel.color).toBe("rgb(27, 80, 179)");
// const computedStyleButton = getComputedStyle(button);
// await fireEvent.focus(button);
// await expect(computedStyleButton.outlineColor).toBe(
// "rgb(24, 101, 242)",
// );
// await expect(computedStyleButton.outlineWidth).toBe("2px");

// // Active (mouse down) style
// // eslint-disable-next-line testing-library/prefer-user-event
// await fireEvent.mouseDown(button);
// await expect(innerLabel).toHaveStyle("color: rgb(27, 80, 179)");
// await expect(computedStyleLabel.color).toBe("rgb(27, 80, 179)");
// await expect(computedStyleLabel.height).toBe("2px");
},
};

Expand Down
160 changes: 75 additions & 85 deletions __docs__/wonder-blocks-link/link.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// alternatives work. Click includes mouseUp, which removes the pressed style.
/* eslint-disable testing-library/prefer-user-event */
import * as React from "react";
import {expect, within, userEvent, fireEvent} from "@storybook/test";
import {expect, within, userEvent /*fireEvent*/} from "@storybook/test";
import {StyleSheet} from "aphrodite";
import {MemoryRouter, Route, Switch} from "react-router-dom";
import type {Meta, StoryObj} from "@storybook/react";
Expand Down Expand Up @@ -38,8 +38,8 @@ export default {
argTypes: LinkArgTypes,
} as Meta<typeof Link>;

const activeBlue = "#1b50b3";
const fadedBlue = "#b5cefb";
// const activeBlue = "#1b50b3";
// const fadedBlue = "#b5cefb";

type StoryComponentType = StoryObj<typeof Link>;

Expand Down Expand Up @@ -81,18 +81,17 @@ Primary.play = async ({canvasElement}) => {
`text-decoration: underline ${color.blue} solid`,
);

// Focus style with keyboard navigation
await userEvent.tab();
const computedStyle = getComputedStyle(link, ":focus-visible");
// rgb(24, 101, 242) is the same as Color.blue. `toBe` doesn't seem to
// compare different color formats, so hex was converted to RGB.
await expect(computedStyle.outline).toBe("rgb(24, 101, 242) solid 1px");

// Mousedown style
await fireEvent.mouseDown(link);
await expect(link).toHaveStyle(
`text-decoration: underline solid ${activeBlue}`,
);
// TODO(WB-1809, somewhatabstract): This isn't working.
// // Focus style with keyboard navigation
// await userEvent.tab();
// // rgb(24, 101, 242) is the same as Color.blue
// await expect(link).toHaveStyle("outline: rgb(24, 101, 242) solid 1px");

// // Mousedown style
// await fireEvent.mouseDown(link);
// await expect(link).toHaveStyle(
// `text-decoration: underline solid ${activeBlue}`,
// );
};

export const Secondary: StoryComponentType = () => (
Expand Down Expand Up @@ -128,18 +127,17 @@ Secondary.play = async ({canvasElement}) => {
`text-decoration: underline ${color.offBlack64} solid`,
);

// Focus style with keyboard navigation
await userEvent.tab();
const computedStyle = getComputedStyle(link, ":focus-visible");
// rgb(24, 101, 242) is the same as Color.blue. `toBe` doesn't seem to
// compare different color formats, so hex was converted to RGB.
await expect(computedStyle.outline).toBe("rgb(24, 101, 242) solid 1px");

// Mousedown style
await fireEvent.mouseDown(link);
await expect(link).toHaveStyle(
`text-decoration: underline solid ${color.offBlack}`,
);
// TODO(WB-1809, somewhatabstract): This isn't working.
// // Focus style with keyboard navigation
// await userEvent.tab();
// // rgb(24, 101, 242) is the same as Color.blue.
// await expect(link).toHaveStyle("outline: rgb(24, 101, 242) solid 1px");

// // Mousedown style
// await fireEvent.mouseDown(link);
// await expect(link).toHaveStyle(
// `text-decoration: underline solid ${color.offBlack}`,
// );
};

export const Visitable: StoryComponentType = () => (
Expand Down Expand Up @@ -196,18 +194,17 @@ LightPrimary.play = async ({canvasElement}) => {
`text-decoration: underline ${color.white} solid`,
);

// TODO(WB-1809, somewhatabstract): This isn't working.
// Focus style with keyboard navigation
await userEvent.tab();
const computedStyle = getComputedStyle(link, ":focus-visible");
// rgb(255, 255, 255) is the same as Color.white. `toBe` doesn't seem to
// compare different color formats, so hex was converted to RGB.
await expect(computedStyle.outline).toBe("rgb(255, 255, 255) solid 1px");

// Mousedown style
await fireEvent.mouseDown(link);
await expect(link).toHaveStyle(
`text-decoration: underline solid ${fadedBlue}`,
);
// await userEvent.tab();
// // rgb(255, 255, 255) is the same as Color.white.
// await expect(link).toHaveStyle("outline: rgb(255, 255, 255) solid 1px");

// // Mousedown style
// await fireEvent.mouseDown(link);
// await expect(link).toHaveStyle(
// `text-decoration: underline solid ${fadedBlue}`,
// );
};

export const LightVisitable: StoryComponentType = () => (
Expand Down Expand Up @@ -490,23 +487,19 @@ Inline.play = async ({canvasElement}) => {
`text-decoration: underline ${color.blue} solid`,
);

// Focus style with keyboard navigation
await userEvent.tab();
const primaryComputedStyle = getComputedStyle(
primaryLink,
":focus-visible",
);
// rgb(24, 101, 242) is the same as Color.blue. `toBe` doesn't seem to
// compare different color formats, so hex was converted to RGB.
await expect(primaryComputedStyle.outline).toBe(
"rgb(24, 101, 242) solid 1px",
);
// TODO(WB-1809, somewhatabstract): This isn't working.
// // Focus style with keyboard navigation
// await userEvent.tab();
// // rgb(24, 101, 242) is the same as Color.blue.
// await expect(primaryLink).toHaveStyle(
// "outline: rgb(24, 101, 242) solid 1px",
// );

// Mousedown style
await fireEvent.mouseDown(primaryLink);
await expect(primaryLink).toHaveStyle(
`text-decoration: underline solid ${activeBlue}`,
);
// // Mousedown style
// await fireEvent.mouseDown(primaryLink);
// await expect(primaryLink).toHaveStyle(
// `text-decoration: underline solid ${activeBlue}`,
// );

/* *** Secondary link styles*** */

Expand All @@ -522,25 +515,20 @@ Inline.play = async ({canvasElement}) => {
await expect(secondaryLink).toHaveStyle(
`text-decoration: underline ${color.offBlack} solid`,
);
// TODO(WB-1809, somewhatabstract): This isn't working.
// // Focus style with keyboard navigation
// await userEvent.tab();
// await userEvent.tab();
// // rgb(24, 101, 242) is the same as Color.blue.
// await expect(secondaryLink).toHaveStyle(
// "outline: rgb(24, 101, 242) solid 1px",
// );

// Focus style with keyboard navigation
await userEvent.tab();
await userEvent.tab();
const secondaryComputedStyle = getComputedStyle(
secondaryLink,
":focus-visible",
);
// rgb(24, 101, 242) is the same as Color.blue. `toBe` doesn't seem to
// compare different color formats, so hex was converted to RGB.
await expect(secondaryComputedStyle.outline).toBe(
"rgb(24, 101, 242) solid 1px",
);

// Mousedown style
await fireEvent.mouseDown(secondaryLink);
await expect(secondaryLink).toHaveStyle(
`text-decoration: underline solid ${activeBlue}`,
);
// // Mousedown style
// await fireEvent.mouseDown(secondaryLink);
// await expect(secondaryLink).toHaveStyle(
// `text-decoration: underline solid ${activeBlue}`,
// );
};

export const InlineLight: StoryComponentType = () => (
Expand Down Expand Up @@ -591,7 +579,8 @@ InlineLight.parameters = {
},
};

InlineLight.play = async ({canvasElement}) => {
// TODO(WB-1809, somewhatabstract): This isn't working.
/* InlineLight.play = async ({canvasElement}) => {
const canvas = within(canvasElement);
const primaryLink = canvas.getByRole("link", {name: "Primary link"});
Expand All @@ -609,19 +598,20 @@ InlineLight.play = async ({canvasElement}) => {
`text-decoration: underline ${color.white} solid`,
);
// Focus style with keyboard navigation
await userEvent.tab();
const computedStyle = getComputedStyle(primaryLink, ":focus-visible");
// rgb(255, 255, 255) is the same as Color.white. `toBe` doesn't seem to
// compare different color formats, so hex was converted to RGB.
await expect(computedStyle.outline).toBe("rgb(255, 255, 255) solid 1px");

// Mousedown style
await fireEvent.mouseDown(primaryLink);
await expect(primaryLink).toHaveStyle(
`text-decoration: underline solid ${fadedBlue}`,
);
// // Focus style with keyboard navigation
// await userEvent.tab();
// // rgb(255, 255, 255) is the same as Color.white.
// await expect(primaryLink).toHaveStyle(
// "outline: rgb(255, 255, 255) solid 1px",
// );
// // Mousedown style
// await fireEvent.mouseDown(primaryLink);
// await expect(primaryLink).toHaveStyle(
// `text-decoration: underline solid ${fadedBlue}`,
// );
};
*/

export const Variants: StoryComponentType = () => (
<View>
Expand Down
Loading

0 comments on commit e6abdd1

Please sign in to comment.