Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update react monorepo to v19 (major) #290

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
"@tsconfig/vite-react": "^3.0.0",
"@types/eslint": "^9.0.0",
"@types/node": "^22.0.0",
"@types/react": "^18.2.21",
"@types/react": "^19.0.0",
"@typescript-eslint/eslint-plugin": "^8.0.0",
"@typescript-eslint/parser": "^8.0.0",
"@vector-im/compound-design-tokens": "^2.0.0",
Expand All @@ -86,8 +86,8 @@
"eslint-plugin-storybook": "^0.11.0",
"jsdom": "^25.0.0",
"prettier": "3.4.2",
"react": "^18.2.0",
"react-dom": "^18.3.1",
"react": "^19.0.0",
"react-dom": "^19.0.0",
"resize-observer-polyfill": "^1.5.1",
"rimraf": "^6.0.0",
"serve": "^14.2.1",
Expand All @@ -111,15 +111,14 @@
"@radix-ui/react-separator": "^1.1.0",
"@radix-ui/react-slot": "^1.1.0",
"classnames": "^2.5.1",
"ts-xor": "^1.3.0",
"vaul": "^1.0.0"
},
"peerDependencies": {
"@fontsource/inconsolata": "^5",
"@fontsource/inter": "^5",
"@types/react": "*",
"@vector-im/compound-design-tokens": ">=1.6.1 <3.0.0",
"react": "^18"
"react": "^18 || ^19.0.0"
},
"peerDependenciesMeta": {
"@types/react": {
Expand Down
12 changes: 4 additions & 8 deletions src/components/Avatar/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,16 @@ limitations under the License.
*/

import classnames from "classnames";
import React, { forwardRef } from "react";
import React, { ComponentProps, forwardRef } from "react";
import { getInitialLetter } from "../../utils/string";
import { SuspenseImg } from "../../utils/SuspenseImg";
import styles from "./Avatar.module.css";
import { useIdColorHash } from "./useIdColorHash";

type AvatarProps = (
| JSX.IntrinsicElements["button"]
| JSX.IntrinsicElements["span"]
) & {
type AvatarProps = (ComponentProps<"button"> | ComponentProps<"span">) & {
/**
* The avatar image URL, if any.
*/
src?: React.ComponentProps<typeof SuspenseImg>["src"];
src?: React.ComponentProps<"img">["src"];
/**
* The Matrix ID, Room ID, or Alias to generate the color when no image source
* is provided. Also used as a fallback when name is empty.
Expand Down Expand Up @@ -62,7 +58,7 @@ type AvatarProps = (
/**
* Callback when the image has failed to load.
*/
onError?: React.ComponentProps<typeof SuspenseImg>["onError"];
onError?: React.ComponentProps<"img">["onError"];
};

/**
Expand Down
2 changes: 1 addition & 1 deletion src/components/Button/IconButton/IconButton.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const WithSubtleBackground: Story = {

export const WithLabel: Story = {
args: {
label: "label",
tooltip: "label",
},
};

Expand Down
4 changes: 1 addition & 3 deletions src/components/Button/IconButton/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ import React, { PropsWithChildren, forwardRef } from "react";
import classnames from "classnames";

import styles from "./IconButton.module.css";
import { UnstyledButton } from "../UnstyledButton";
import { UnstyledButtonPropsFor } from "../UnstyledButton";
import { UnstyledButton, UnstyledButtonPropsFor } from "../UnstyledButton";
import { IndicatorIcon } from "../../Icon/IndicatorIcon/IndicatorIcon";
import { Tooltip } from "../../Tooltip/Tooltip";

Expand Down Expand Up @@ -53,7 +52,6 @@ type IconButtonProps = UnstyledButtonPropsFor<"button"> & {
*/
tooltip?: string;
subtleBackground?: boolean;
label?: string;
};

/**
Expand Down
4 changes: 2 additions & 2 deletions src/components/Dropdown/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,10 @@ const DropdownItem = memo(function DropdownItem({
function useOpen(): [
boolean,
Dispatch<SetStateAction<boolean>>,
RefObject<HTMLDivElement>,
RefObject<HTMLDivElement | null>,
] {
const [open, setOpen] = useState(false);
const ref = useRef<HTMLDivElement>(null);
const ref = useRef<HTMLDivElement | null>(null);

// If the user clicks outside the dropdown, close it
useEffect(() => {
Expand Down
4 changes: 3 additions & 1 deletion src/components/Form/Controls/EditInPlace/EditInPlace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,9 @@ export const EditInPlace = forwardRef<HTMLInputElement, Props>(
const shouldShowSaveButton =
state === State.Dirty || state === State.Saving || isFocusWithin;

const hideTimer = useRef<ReturnType<typeof setTimeout>>();
const hideTimer = useRef<ReturnType<typeof setTimeout> | undefined>(
undefined,
);

useEffect(() => {
// Start a timer when we switch to the saved state
Expand Down
2 changes: 1 addition & 1 deletion src/components/Menu/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const MenuItem = <C extends MenuItemElement = "button">({
onClick: onClickProp,
disabled,
...props
}: Props<C>): JSX.Element => {
}: Props<C>): React.ReactElement => {
const Component = as ?? ("button" as ElementType);
const context = useContext(MenuContext);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,10 @@ function ReleaseAnnouncementAnchor({
children,
context.getReferenceProps({
ref,
...children.props,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cloneElement will anyway keep the existing props

// If the ReleaseAnnouncement is open, we need manually aria-describedby.
// The RA has the dialog role and it's not adding automatically the aria-describedby.
...(context.open && {
"aria-describedby": context.getFloatingProps().id,
"aria-describedby": context.getFloatingProps().id as string,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getFloatingProps returns Record<string, unknown>, but we know this is a string

}),
}),
);
Expand Down
5 changes: 4 additions & 1 deletion src/components/Tooltip/Tooltip.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,17 @@ const meta = {
args: {
// needed, to prevent the tooltip to be in controlled mode
onOpenChange: undefined,
open: undefined,
description: "",
label: "",
children: (
<IconButton>
<UserIcon />
</IconButton>
),
},
decorators: [
(Story: StoryFn) => (
(Story) => (
<div style={{ padding: 100 }}>
<TooltipProvider>
<Story />
Expand Down
132 changes: 97 additions & 35 deletions src/components/Tooltip/Tooltip.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,12 @@ import { describe, it, expect, vi } from "vitest";
import { fireEvent, render, screen } from "@testing-library/react";
import React, { act } from "react";

import * as stories from "./Tooltip.stories";
import { composeStories, composeStory } from "@storybook/react";
import { IconButton } from "../Button";
import userEvent from "@testing-library/user-event";
import { TooltipProvider } from "./TooltipProvider";
import { Tooltip } from "./Tooltip";
import { UserIcon } from "@vector-im/compound-design-tokens/assets/web/icons";

const {
Default,
WithStringCaption,
WithComponentCaption,
ForcedOpen,
ForcedClose,
ForcedDisabled,
InteractiveTrigger,
NonInteractiveTrigger,
Descriptive,
} = composeStories(stories);

/**
* Patches an element to always match :focus-visible whenever it's in focus.
* JSDOM doesn't seem to support this selector on its own.
Expand All @@ -52,47 +39,87 @@ function mockFocusVisible(e: Element): void {

describe("Tooltip", () => {
it("renders open by default", () => {
render(<ForcedOpen />);
render(
<TooltipProvider>
<Tooltip open={true} label="I'm always open">
<IconButton>
<UserIcon />
</IconButton>
</Tooltip>
</TooltipProvider>,
);
// tooltip labels button and does not use role="tooltip"
screen.getByRole("button", { name: "I'm always open" });
expect(screen.queryByRole("tooltip")).toBe(null);
});

it("renders closed by default", () => {
render(<ForcedClose />);
render(
<TooltipProvider>
<Tooltip open={false} description="You can't see me">
<span>No tooltip to see here</span>
</Tooltip>
</TooltipProvider>,
);
// no tooltip, and no label either
expect(screen.queryByRole("tooltip")).toBe(null);
expect(screen.queryByRole("button", { name: /.*/ })).toBe(null);
});

it("renders disabled", () => {
render(<ForcedDisabled />);
render(
<TooltipProvider>
<Tooltip disabled={true} description="You can't see me">
<span>No tooltip to see here</span>
</Tooltip>
</TooltipProvider>,
);
// no tooltip, and no label either
expect(screen.queryByRole("tooltip")).toBe(null);
expect(screen.queryByRole("button", { name: /.*/ })).toBe(null);
});

it("renders default tooltip", async () => {
render(<Default />);
render(
<TooltipProvider>
<Tooltip label="@bob:example.org">
<IconButton>
<UserIcon />
</IconButton>
</Tooltip>
</TooltipProvider>,
);
// tooltip labels button and does not use role="tooltip"
screen.getByRole("button", { name: "@bob:example.org" });
expect(screen.queryByRole("tooltip")).toBe(null);
});

it("opens tooltip on focus", async () => {
const user = userEvent.setup();
render(<InteractiveTrigger />);
render(
<TooltipProvider>
<Tooltip isTriggerInteractive={true} description="Description">
<a href="https://example.org">Link</a>
</Tooltip>
</TooltipProvider>,
);
mockFocusVisible(screen.getByRole("link"));
expect(screen.queryByRole("tooltip")).toBe(null);
await user.tab();
// trigger focused, tooltip shown
expect(screen.getByRole("link")).toHaveFocus();
screen.getByRole("tooltip");
await screen.findByRole("tooltip");
});

it("opens tooltip on focus where trigger is non interactive", async () => {
const user = userEvent.setup();
render(<NonInteractiveTrigger />);
render(
<TooltipProvider>
<Tooltip isTriggerInteractive={false} description="Description">
<span>Just some text</span>
</Tooltip>
</TooltipProvider>,
);
mockFocusVisible(screen.getByText("Just some text").parentElement!);
expect(screen.queryByRole("tooltip")).toBe(null);
await user.tab();
Expand All @@ -104,7 +131,13 @@ describe("Tooltip", () => {
it("opens tooltip on long press", async () => {
vi.useFakeTimers();
try {
render(<InteractiveTrigger />);
render(
<TooltipProvider>
<Tooltip isTriggerInteractive={true} description="Description">
<a href="https://example.org">Link</a>
</Tooltip>
</TooltipProvider>,
);
expect(screen.queryByRole("tooltip")).toBe(null);
// Press
fireEvent.touchStart(screen.getByRole("link"));
Expand All @@ -125,25 +158,33 @@ describe("Tooltip", () => {

it("overrides default tab index for non interactive triggers", async () => {
const user = userEvent.setup();
const Component = composeStory(
{
...stories.NonInteractiveTrigger,
args: {
...stories.NonInteractiveTrigger.args,
nonInteractiveTriggerTabIndex: -1,
},
},
stories.default,
render(
<TooltipProvider>
<Tooltip
isTriggerInteractive={false}
nonInteractiveTriggerTabIndex={-1}
description="Description"
>
<span>Just some text</span>
</Tooltip>
</TooltipProvider>,
);
render(<Component />);
await user.tab();
// trigger cannot be focused
expect(screen.queryByRole("tooltip")).toBe(null);
});

it("renders with string caption", async () => {
const user = userEvent.setup();
render(<WithStringCaption />);
render(
<TooltipProvider>
<Tooltip label="I can have a caption" caption="My beautiful caption">
<IconButton>
<UserIcon />
</IconButton>
</Tooltip>
</TooltipProvider>,
);
await user.tab();
// tooltip labels button and describes button with caption
expect(
Expand All @@ -153,7 +194,22 @@ describe("Tooltip", () => {

it("renders with component caption", async () => {
const user = userEvent.setup();
render(<WithComponentCaption />);
render(
<TooltipProvider>
<Tooltip
label="Copy"
caption={
<>
<kbd>Ctrl</kbd> + <kbd>C</kbd>
</>
}
>
<IconButton>
<UserIcon />
</IconButton>
</Tooltip>
</TooltipProvider>,
);
await user.tab();
// tooltip labels button and describes button with caption
expect(
Expand All @@ -162,7 +218,13 @@ describe("Tooltip", () => {
});

it("renders a descriptive tooltip", async () => {
render(<Descriptive />);
render(
<TooltipProvider>
<Tooltip open={true} description="Employer Identification Number">
<span>EIN</span>
</Tooltip>
</TooltipProvider>,
);
// tooltip shown, but does not change the button's accessible name
screen.getByRole("tooltip", { name: "Employer Identification Number" });
expect(screen.queryByRole("button", { name: "EIN" })).toBe(null);
Expand Down
Loading
Loading