Skip to content

[EuiResizeObserver] Remove reflow/getBoundingClientRect() usage on resize #7575

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

Merged
cee-chen merged 3 commits into from
Mar 13, 2024
Merged

[EuiResizeObserver] Remove reflow/getBoundingClientRect() usage on resize #7575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c3eb4b3
Update resize observer to use `borderBoxSize` instead of `getBounding…
cee-chen Mar 12, 2024
6df1915
changelog
cee-chen Mar 12, 2024
b5cdda0
[hook] Remove initial `getBoundingClientRect` on mount
cee-chen Mar 13, 2024
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
1 change: 1 addition & 0 deletions changelogs/upcoming/7575.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Enhanced `EuiResizeObserver` and `useResizeObserver`'s performance to not trigger page reflows on resize event
28 changes: 7 additions & 21 deletions src/components/observer/resize_observer/resize_observer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,9 @@ export class EuiResizeObserver extends EuiObserver<EuiResizeObserverProps> {
width: 0,
};

onResize: ResizeObserverCallback = () => {
// `entry.contentRect` provides incomplete `height` and `width` data.
// Use `getBoundingClientRect` to account for padding and border.
// https://developer.mozilla.org/en-US/docs/Web/API/DOMRectReadOnly
if (!this.childNode) return;
const { height, width } = this.childNode.getBoundingClientRect();
onResize: ResizeObserverCallback = ([entry]) => {
const { inlineSize: width, blockSize: height } = entry.borderBoxSize[0];

// Check for actual resize event
if (this.state.height === height && this.state.width === width) {
return;
Expand Down Expand Up @@ -94,22 +91,11 @@ export const useResizeObserver = (

useEffect(() => {
if (container != null) {
// ResizeObserver's first call to the observation callback is scheduled in the future
// so find the container's initial dimensions now
const boundingRect = container.getBoundingClientRect();
setSize({
width: boundingRect.width,
height: boundingRect.height,
});
Comment on lines -97 to -103
Copy link
Contributor Author

@cee-chen cee-chen Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I've opted to remove this one-off getBoundingClientRect() call on mount because it appears to only have been added purely for EuiDataGrid (#2991 (review)) and to be completely frank, at this point EuiDataGrid has a lot more performance issues going on than a quick flash of unsized columns. The extra reflow/performance hit is, IMO, not worth it 🤷

Copy link
Contributor

@mgadewoll mgadewoll Mar 13, 2024

Choose a reason for hiding this comment

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

Is this still happening? I tried to reproduce this in the EUI docs but I had the same result on the feature branch and on production. (but maybe I'm missing some context on how to reproduce it).

In any case, I'm ok with removing it. We can keep an eye out if there will be issues raised because of it.

Copy link
Contributor Author

@cee-chen cee-chen Mar 13, 2024

Choose a reason for hiding this comment

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

I wasn't able to repro either but I wasn't trying super hard or analyzing frame by frame like Dave apparently was 😅 EuiDataGrid has changed a lot since then so def possible it either isn't an issue or simply isn't worth it anymore!


const observer = makeResizeObserver(container, () => {
// `entry.contentRect` provides incomplete `height` and `width` data.
// Use `getBoundingClientRect` to account for padding and border.
// https://developer.mozilla.org/en-US/docs/Web/API/DOMRectReadOnly
const { height, width } = container.getBoundingClientRect();
const observer = makeResizeObserver(container, ([entry]) => {
const { inlineSize, blockSize } = entry.borderBoxSize[0];
setSize({
width,
height,
width: inlineSize,
height: blockSize,
});
});

Expand Down