Skip to content
This repository has been archived by the owner on Jan 5, 2023. It is now read-only.

Add useWindowSize hook to DropdownButton #75

Closed
wants to merge 1 commit into from

Conversation

mayank99
Copy link
Contributor

@mayank99 mayank99 commented May 21, 2021

Hook listens to window resize event and updates size. Used in DropdownButton to update menu width when window gets resized.

Checklist

  • Add meaningful tests for your component (verify that all lines are covered)
  • Verify that all existing tests pass
  • Add component features demo in Storybook (different stories)
  • Add an entry to the changelog for any new components and changes
  • Add screenshots of the key elements of the component below
  • Add any additional comments below describing the features you've implemented

Additional Comments

After #73, it became obvious that the header dropdowns were not being updated on resizing window so this PR is mainly intended to fix that.

const [menuWidth, setMenuWidth] = React.useState(0);
const ref = React.useRef<HTMLButtonElement>(null);

React.useEffect(() => {
if (ref.current) {
setMenuWidth(ref.current.offsetWidth);
}
}, [children, size, styleType]);
}, [children, size, styleType, windowSize]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really needed? Cause I looked at other libs and no one is doing this. Also, it just adds additional complexity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm idk, it's nice. Which libs did you check? Most of them are meant to be as customizable as possible, rather than enforcing standards.

I guess we could also just provide a util.

https://material-ui.com/components/use-media-query/
https://material-ui.com/components/textarea-autosize/
https://blueprintjs.com/docs/#popover2-package/resize-sensor2
https://material.angular.io/cdk/layout/overview

Copy link
Contributor

Choose a reason for hiding this comment

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

antd and material-ui changes the select element and menu stays the same size when resizing. react-bootstrap and blueprint does not change anything on resize so hard to tell.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Material likes to keep the dropdown width same as select, but it does not have any example where select is taking full width.
image

I would think if you were to code a dynamic (resizable) dropdown using Material, you'd want the width to respond to the change.

But AntD does have some examples.
antd-select

Copy link
Contributor Author

@mayank99 mayank99 May 25, 2021

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Your AntD gif does the same as we do right now 😀 and if understand correctly your hook will work with opened menu and resized window.
That modifier would solve a lot of duplication. But does it handle live resizes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is our current behavior and really the main thing I'm trying to solve.
resize

And I haven't gotten that modifier to work tbh.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is a bad behavior:
image
Maybe @FlyersPh9 can comment on this one.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants