marigold-ui / marigold

Design System based on react-aria and Tailwind CSS
https://marigold-ui.io
MIT License
101 stars 7 forks source link

feat(components): improve <Select> with react-aria #1147

Closed ti10le closed 2 years ago

ti10le commented 2 years ago

closes #1146

changeset-bot[bot] commented 2 years ago

⚠️ No Changeset found

Latest commit: 1e52db26ac5b586061e4ed652581dfb6b73e6dca

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mrgld/marigold/yGPwGUoEws7n5r8ccRLL1g8xhwmu
✅ Preview: https://marigold-git-improve-select-component-mrgld.vercel.app

[Deployment for 75df66d failed]

netlify[bot] commented 2 years ago

:x: Deploy Preview for keen-wiles-f4b73a failed.

:hammer: Explore the source changes: 1e3d734e6084f6933131c86de564acf5f49b7580

:mag: Inspect the deploy log: https://app.netlify.com/sites/keen-wiles-f4b73a/deploys/60e45caab568e000078421eb

coveralls commented 2 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 1e52db26ac5b586061e4ed652581dfb6b73e6dca on improve-select-component into f3f61456afa65ddeec8c9f1346c439bd91f3aa12 on main.

ti10le commented 2 years ago

wrong button

do this if the aria button is merged

sebald commented 2 years ago

Bug 🥲 image

Also: The select "jumps" if opned.

ti10le commented 2 years ago

The issue remains the same. The select (or the popup to be specific) is shifting the layout, which it should not do. I changed the example to show what I mean:

image

This is what a user would expect:

image

PS: The select still jumps because of the focus ring or something else?

But in the second picture that the CodeBlock grow by the listbox, right?

sebald commented 2 years ago

Don't quite understand what you mean 😐

ti10le commented 2 years ago

Don't quite understand what you mean 😐

Basically, the listbox is in the foreground above everything. (Actually like on your second picture) But if the select is opened in the codeBlock then the block should increase, right?

sebald commented 2 years ago

No, why should it? 🤔 The listbox doesn't change the underlying layout.

ti10le commented 2 years ago

No, why should it? 🤔 The listbox doesn't change the underlying layout.

Ok, this was my opinion too but then I understood something wrong in our last chat. Now everything clear :)