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

Use ResizeObserver to calculate grid width #2129

Merged
merged 7 commits into from
Aug 24, 2020
Merged

Conversation

amanmahajan7
Copy link
Contributor

ResizeObserver has a pretty good browser support now
https://caniuse.com/#feat=resizeobserver
https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver

This can solve a few issues

  • Grid container resize may not always trigger window resize but resize observer will catch it
  • If the grid is initially hidden and then displayed then grid width will be calculated again

TS does not have definitions for ResizeObserver so I copied some from https://www.w3.org/TR/resize-observer/#resizeobserversize

@amanmahajan7 amanmahajan7 self-assigned this Aug 24, 2020
return null;
}

return new ResizeObserver((entries?: ResizeObserverEntry[]) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if entries can be undefined or not so added a safeguard


return new ResizeObserver((entries?: ResizeObserverEntry[]) => {
if (Array.isArray(entries) && entries.length > 0) {
const newWidth = entries[0].borderBoxSize?.inlineSize ?? entries[0].target.getBoundingClientRect().width;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

borderBoxSize is not supported everywhere and it is marked as experimental
https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserverEntry/borderBoxSize

src/hooks/useGridWidth.ts Outdated Show resolved Hide resolved
src/hooks/useGridWidth.ts Outdated Show resolved Hide resolved
src/hooks/useGridWidth.ts Outdated Show resolved Hide resolved
src/hooks/useGridWidth.ts Outdated Show resolved Hide resolved
src/hooks/useGridWidth.ts Outdated Show resolved Hide resolved
src/hooks/useGridWidth.ts Outdated Show resolved Hide resolved
setGridWidth(gridRef.current!.getBoundingClientRect().width);
}

// Fallback to window resize
onResize();
Copy link
Contributor

Choose a reason for hiding this comment

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

Will the resize observer work immediately? Do we not need to call this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it does seem to work immediately

@nstepien
Copy link
Contributor

Should we remove AutoSizer from the stories?

@nstepien
Copy link
Contributor

nstepien commented Aug 24, 2020

https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver/observe

box: Sets which box model the observer will observe changes to. Possible values are content-box (the default), and border-box.

https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver

Note: The content box is the box in which content can be placed, meaning the border box minus the padding and border width. The border box encompasses the content, padding, and border. See The box model for further explanation.

Maybe we should use the border-box since we do width - border widths, or use the content-box and remove the border widths from the equation. Although we shouldn't rely on getBoundingClientRect() in that case.

@amanmahajan7
Copy link
Contributor Author

Should we remove AutoSizer from the stories?

We need it for the height calculation

src/hooks/useGridWidth.ts Show resolved Hide resolved
amanmahajan7 and others added 2 commits August 24, 2020 06:53
Co-authored-by: Nicolas Stepien <567105+nstepien@users.noreply.github.com>
@amanmahajan7 amanmahajan7 merged commit 7dbc865 into canary Aug 24, 2020
@amanmahajan7 amanmahajan7 deleted the am-resize-observer branch August 24, 2020 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants