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

Remove space from combobox single select #3384

Merged
merged 9 commits into from
Nov 29, 2024

Conversation

lotorvik
Copy link
Contributor

Description

On combobox and single select you have a space between a selected value and the input. This makes sense for multi select where you have multiple chips and want to add more chips. But for single select it looks strange and when I press backspace I expect the space to go away but the whole value gets removed.

This PR suggest to remove the gap between the input and the selected value with single select on combobox. This makes it behave more like native select.

Copy link

changeset-bot bot commented Nov 19, 2024

🦋 Changeset detected

Latest commit: 5d8b1cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@navikt/ds-react Patch
@navikt/ds-css Patch
@navikt/aksel-stylelint Patch
@navikt/aksel Patch
@navikt/ds-tokens Patch
@navikt/ds-tailwind Patch
@navikt/aksel-icons Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link
Contributor

github-actions bot commented Nov 19, 2024

Storybook demo / Chromatic

📝 Endringer til review: 1

68d316e2f | 91 komponenter | 135 stories

@it-vegard
Copy link
Contributor

My only worry here is that it looks like the selected value is part of the editable text, meaning the user could expect to press backspace to remove only the last character (instead of the entire selected value), or keep extending the value by typing more characters (instead of replacing the selected value). 🤔

@JulianNymark
Copy link
Contributor

recommend waiting a tiny bit (maybe less than 24 hours) before merging this, since the rewrite for combobox css (new tokens) is 99% done. Maybe do this in the same rewrite PR 🤔 (if we decide to go with the change).

@lotorvik
Copy link
Contributor Author

I see your point @it-vegard . But with the space there I expect the whitespace to go away and the whole text gets removed, so I don't feel this is worse. But you might be correct that this should be solved diffrently.

I checked with a couple of other searchable selects how they solved it.

  • The popular React Select places the cursor in front of the text, but totally ignores the text inside the box. Not sure how much I like this solution, I did not expect the text to go away when I wrote. Only good thing is that you do not need to use backspace before searching again.
  • Atlassian select works same as React Select. Did not check if they actually use React Select.
  • The design system for Fremtind forsikring (https://jokul.fremtind.no/komponenter/select/) looked like a good solution where they just removes the content when the select get focus, since that is actually what you would like to do. The only problem with this solution is that I did not find a way to actually remove the value from the select...

@KenAJoh KenAJoh requested a review from HalvorHaugan November 27, 2024 12:45
@navikt/core/css/form/combobox.css Outdated Show resolved Hide resolved
@navikt/core/css/form/combobox.css Outdated Show resolved Hide resolved
.changeset/yellow-days-rhyme.md Outdated Show resolved Hide resolved
@KenAJoh KenAJoh enabled auto-merge (squash) November 29, 2024 12:54
@KenAJoh KenAJoh merged commit fa2904d into main Nov 29, 2024
4 checks passed
@KenAJoh KenAJoh deleted the bugfix/combobox-single-select-remove-gap branch November 29, 2024 14:03
@github-actions github-actions bot mentioned this pull request Nov 29, 2024
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.

5 participants