-
Notifications
You must be signed in to change notification settings - Fork 42
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
[Combobox] Only open dropdown onClick, not onFocus. #3440
Conversation
…wn list and hide error
🦋 Changeset detectedLatest commit: e80e933 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
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 |
Storybook demoa46619918 | 91 komponenter | 135 stories |
The list no longer closes after selecting an option in single select mode. |
@HalvorHaugan Should be fixed now 🤞 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
if (inputProps.disabled || readOnly) { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we technically need to check this, but it doesn't hurt either.
.changeset/forty-rules-ring.md
Outdated
@@ -0,0 +1,5 @@ | |||
--- | |||
"@navikt/ds-react": minor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would patch
be more correct here? 🤔 Not sure myself.
Description
This solves the issue where programmatic focus would open dropdown. This in turn would hide any error for the field in the cases that would be relevant.
Resolves #3437
The fix itself almost seems too simple to just "work", so will need some help reviewing edgecases that might have been forgotten. I didn't actually need to check for readOnly or disabled-state for it to work, but exit early anyways just incase we change that in the future.
Component Checklist 📝
@navikt/core/css/config/_mappings.js
)@navikt/core/css/tokens.json
)@navikt/aksel-stylelint/src/deprecations.ts
)@navikt/core/react/src/index.ts
and@navikt/core/react/package.json
)@navikt/core/css/index.css
)<Component>: <gitmoji?> <Text>.
E.g. "Button: ✨ Add feature xyz.")