Skip to content

fix: wrap tether instance creation and updates in requestAnimationFrame #236

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.

Sign up for GitHub

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

Open
SBoudrias wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: wrap tether instance creation and updates in requestAnimationFrame #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
31 changes: 20 additions & 11 deletions src/react-tether.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,13 @@ export default class ReactTether extends Component<ReactTetherProps> {
this._destroy();
}

this._tetherInstance = new Tether(tetherOptions);
this._registerEventListeners();
// Use requestAnimationFrame to defer tether instance creation until after
// the browser has completed layout. This prevents positioning issues that
// can occur when the DOM hasn't fully settled.
requestAnimationFrame(() => {
this._tetherInstance = new Tether(tetherOptions);
this._registerEventListeners();
});
}

_destroyTetherInstance() {
Expand Down Expand Up @@ -277,15 +282,19 @@ export default class ReactTether extends Component<ReactTetherProps> {
}
}

this._addContainerToDOM();

if (this._tetherInstance) {
this._tetherInstance.setOptions(tetherOptions);
} else {
this._createTetherInstance(tetherOptions);
}

this._tetherInstance?.position();
// Use requestAnimationFrame to defer DOM mutations and tether updates until
// after the browser has completed layout. This prevents positioning issues
// and flickering that can occur during rapid updates.
requestAnimationFrame(() => {
this._addContainerToDOM();

if (this._tetherInstance) {
this._tetherInstance.setOptions(tetherOptions);
this._tetherInstance.position();
} else {
this._createTetherInstance(tetherOptions);
}
});
}

override render() {
Expand Down
136 changes: 89 additions & 47 deletions tests/unit/component.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import React from "react";
import { render, screen } from "@testing-library/react";
import { render, screen, waitFor, cleanup } from "@testing-library/react";
import TetherComponent from "../../src/react-tether";

describe("TetherComponent", () => {
afterEach(() => {
cleanup();
// Clean up any tether elements left in the DOM
document.querySelectorAll(".tether-element").forEach((el) => el.remove());
document.querySelectorAll("[data-tether-id]").forEach((el) => el.remove());
document.querySelectorAll("#test-container").forEach((el) => el.remove());
document.querySelectorAll("#test-container2").forEach((el) => el.remove());
});
it("should render the target", () => {
render(
<TetherComponent
Expand All @@ -14,44 +22,50 @@ describe("TetherComponent", () => {
expect(screen.getByTestId("target")).toBeInTheDocument();
});

it("should render the element", () => {
it("should render the element", async () => {
render(
<TetherComponent
attachment="top left"
renderTarget={(ref) => <div ref={ref} data-testid="target" />}
renderElement={(ref) => <div ref={ref} data-testid="element" />}
/>
);
expect(screen.getByTestId("element")).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByTestId("element")).toBeInTheDocument();
});
});

it("should create a tether element", () => {
it("should create a tether element", async () => {
render(
<TetherComponent
attachment="top left"
renderTarget={(ref) => <div ref={ref} data-testid="target" />}
renderElement={(ref) => <div ref={ref} data-testid="element" />}
/>
);
const tetherElement = document.querySelector(".tether-element");
expect(tetherElement).toBeInTheDocument();
await waitFor(() => {
const tetherElement = document.querySelector(".tether-element");
expect(tetherElement).toBeInTheDocument();
});
});

it("should render the second child in the tether element", () => {
it("should render the second child in the tether element", async () => {
render(
<TetherComponent
attachment="top left"
renderTarget={(ref) => <div ref={ref} data-testid="target" />}
renderElement={(ref) => <div ref={ref} data-testid="element" />}
/>
);
const element = document.querySelector(
".tether-element [data-testid=element]"
);
expect(element).toBeInTheDocument();
await waitFor(() => {
const element = document.querySelector(
".tether-element [data-testid=element]"
);
expect(element).toBeInTheDocument();
});
});

it("should add className to tether element", () => {
it("should add className to tether element", async () => {
render(
<TetherComponent
attachment="top left"
Expand All @@ -60,13 +74,15 @@ describe("TetherComponent", () => {
renderElement={(ref) => <div ref={ref} data-testid="element" />}
/>
);
const tetherElement = document.querySelector(".tether-element");
expect(tetherElement).toBeInTheDocument();
expect(tetherElement?.classList.contains("custom-class-1")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-2")).toBe(true);
await waitFor(() => {
const tetherElement = document.querySelector(".tether-element");
expect(tetherElement).toBeInTheDocument();
expect(tetherElement?.classList.contains("custom-class-1")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-2")).toBe(true);
});
});

it("should swap out classes when className changes", () => {
it("should swap out classes when className changes", async () => {
let { rerender } = render(
<TetherComponent
attachment="top left"
Expand All @@ -75,9 +91,11 @@ describe("TetherComponent", () => {
renderElement={(ref) => <div ref={ref} data-testid="element" />}
/>
);
let tetherElement = document.querySelector(".tether-element");
expect(tetherElement?.classList.contains("custom-class-1")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-2")).toBe(true);
await waitFor(() => {
let tetherElement = document.querySelector(".tether-element");
expect(tetherElement?.classList.contains("custom-class-1")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-2")).toBe(true);
});

rerender(
<TetherComponent
Expand All @@ -88,11 +106,13 @@ describe("TetherComponent", () => {
/>
);

tetherElement = document.querySelector(".tether-element");
expect(tetherElement?.classList.contains("custom-class-1")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-2")).toBe(false);
expect(tetherElement?.classList.contains("custom-class-3")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-4")).toBe(true);
await waitFor(() => {
let tetherElement = document.querySelector(".tether-element");
expect(tetherElement?.classList.contains("custom-class-1")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-2")).toBe(false);
expect(tetherElement?.classList.contains("custom-class-3")).toBe(true);
expect(tetherElement?.classList.contains("custom-class-4")).toBe(true);
});
});

it("should render a just a target", () => {
Expand Down Expand Up @@ -138,7 +158,7 @@ describe("TetherComponent", () => {
expect(document.querySelector(".tether-element")).not.toBeInTheDocument();
});

it("should destroy the tether element if the first/second child is unmounted", () => {
it("should destroy the tether element if the first/second child is unmounted", async () => {
function ToggleComponent({
first,
second,
Expand All @@ -161,9 +181,11 @@ describe("TetherComponent", () => {
let { rerender } = render(<ToggleComponent first second />);

expect(screen.getByTestId("target")).toBeInTheDocument();
expect(
document.querySelector(".tether-element [data-testid=element]")
).toBeInTheDocument();
await waitFor(() => {
expect(
document.querySelector(".tether-element [data-testid=element]")
).toBeInTheDocument();
});

rerender(<ToggleComponent first />);

Expand All @@ -180,7 +202,7 @@ describe("TetherComponent", () => {
).not.toBeInTheDocument();
});

it("allows changing the tether element tag", () => {
it("allows changing the tether element tag", async () => {
render(
<TetherComponent
attachment="top left"
Expand All @@ -189,10 +211,12 @@ describe("TetherComponent", () => {
renderElement={(ref) => <div ref={ref} data-testid="element" />}
/>
);
expect(document.querySelector(".tether-element")?.nodeName).toBe("ASIDE");
await waitFor(() => {
expect(document.querySelector(".tether-element")?.nodeName).toBe("ASIDE");
});
});

it("allows changing the tether element tag on the fly", () => {
it("allows changing the tether element tag on the fly", async () => {
function DifferentTagsComponent({ isAside }: { isAside: boolean }) {
return (
<TetherComponent
Expand All @@ -205,14 +229,18 @@ describe("TetherComponent", () => {
}
let { rerender } = render(<DifferentTagsComponent isAside={false} />);

expect(document.querySelector(".tether-element")?.nodeName).toBe("DIV");
await waitFor(() => {
expect(document.querySelector(".tether-element")?.nodeName).toBe("DIV");
});

rerender(<DifferentTagsComponent isAside={true} />);

expect(document.querySelector(".tether-element")?.nodeName).toBe("ASIDE");
await waitFor(() => {
expect(document.querySelector(".tether-element")?.nodeName).toBe("ASIDE");
});
});

it("allows changing the tether element tag", () => {
it("allows changing the renderElementTo location", async () => {
const container = document.createElement("div");
container.setAttribute("id", "test-container");
// Tether requires the container element to have position static
Expand All @@ -229,12 +257,14 @@ describe("TetherComponent", () => {
);

expect(document.querySelector("#test-container")).toBeInTheDocument();
expect(
document.querySelector("#test-container .tether-element")
).toBeInTheDocument();
await waitFor(() => {
expect(
document.querySelector("#test-container .tether-element")
).toBeInTheDocument();
});
});

it("allows changing the tether element tag on the fly", () => {
it("allows changing the renderElementTo on the fly", async () => {
const container = document.createElement("div");
container.setAttribute("id", "test-container");
// Tether requires the container element to have position static
Expand All @@ -261,16 +291,20 @@ describe("TetherComponent", () => {
);

expect(document.querySelector("#test-container")).toBeInTheDocument();
expect(
document.querySelector("#test-container .tether-element")
).toBeInTheDocument();
await waitFor(() => {
expect(
document.querySelector("#test-container .tether-element")
).toBeInTheDocument();
});

rerender(<DifferentRenderElementToComponent to="#test-container2" />);

expect(document.querySelector("#test-container2")).toBeInTheDocument();
expect(
document.querySelector("#test-container2 .tether-element")
).toBeInTheDocument();
await waitFor(() => {
expect(
document.querySelector("#test-container2 .tether-element")
).toBeInTheDocument();
});
});

// Not sure how to properly test this
Expand All @@ -290,7 +324,7 @@ describe("TetherComponent", () => {
expect(onUpdate).toHaveBeenCalled();
});

it("passes arguments when on onRepositioned() is called", () => {
it("passes arguments when on onRepositioned() is called", async () => {
const onRepositioned = jest.fn();

let { rerender } = render(
Expand All @@ -301,6 +335,12 @@ describe("TetherComponent", () => {
renderElement={(ref) => <div ref={ref} data-testid="element" />}
/>
);

// Wait for initial tether setup
await waitFor(() => {
expect(document.querySelector(".tether-element")).toBeInTheDocument();
});

rerender(
<TetherComponent
attachment="top right"
Expand All @@ -310,6 +350,8 @@ describe("TetherComponent", () => {
/>
);

expect(onRepositioned).toHaveBeenCalled();
await waitFor(() => {
expect(onRepositioned).toHaveBeenCalled();
});
});
});