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

fix: layout shift caused by Dropdownmenu and make it to a separate component #906

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

rajeshg
Copy link
Contributor

@rajeshg rajeshg commented Jan 17, 2025

User drop down menu causes a horizontal layout shift (see shadcn-ui/ui#977)

Also refactored UserDropdown to it's own component similar to SearchBar

Test Plan

Checklist

  • Tests updated
  • Docs updated

Screenshots

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks for this! Just one thing. Also, can you verify that the actual user experience is unchanged?

package.json Outdated
Comment on lines 7 to 8
"head": "5c5328b38e442d78933d5be8fae21fe317296c29",
"date": "2025-01-17T19:16:26Z"
"head": "8468b0101fbee9d09f7ea426eaefec74583641b4",
"date": "2025-01-17T20:19:33Z"
Copy link
Member

Choose a reason for hiding this comment

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

Could you undo this change? Just leave out the package.json from the changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@rajeshg rajeshg force-pushed the main branch 2 times, most recently from 5dfde4f to 8468b01 Compare January 17, 2025 22:03
@rajeshg
Copy link
Contributor Author

rajeshg commented Jan 17, 2025

Thanks for this! Just one thing. Also, can you verify that the actual user experience is unchanged?

Yes. Looks good to me. Tested from Windows env

@kentcdodds kentcdodds merged commit eaa6d60 into epicweb-dev:main Jan 17, 2025
@kentcdodds
Copy link
Member

Thanks!

@kentcdodds
Copy link
Member

The modal={false} prop changed behavior and failed the playwright tests so I'm reverting the modal prop. I don't see any issues with layout shift with things...

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.

2 participants