-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(combo-box): support v10 input controls #2241
fix(combo-box): support v10 input controls #2241
Conversation
3836361
to
0a006e7
Compare
Deploy preview for the-carbon-components ready! Built with commit a5ffad8 https://deploy-preview-2241--the-carbon-components.netlify.com |
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.
c66db66
to
b7e213b
Compare
both points should be addressed now @aagonzales |
aa9fcc2
to
121ca54
Compare
feedback from @carmacleod on the React component: https://github.com/IBM/carbon-components-react/issues/1763#issuecomment-476252889
|
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.
Changes:
- There needs to be a 1px border bottom rule with
ui-04
color when the dropdown is closed.
- There needs to be a 1px border bottom rule with
ui-03
color when the dropdown is open.
-
There needs to be an error field border around the examples that are showing an error state. 2px
support-01
color. -
The close icon needs to be
icon-02
color. -
If you full render, all of the open dropdowns merge into eachother because you can't close them, i don't know if that was intended.
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.
it looks like the text input styles were overriding the listbox styles there, should be fixed @laurenmrice |
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.
Looks good 👍
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 basically, just one thing - Thanks alot @emyarod!
@@ -129,10 +247,50 @@ | |||
</div> | |||
</div> | |||
</div> | |||
<div style="margin-bottom:10rem"></div> |
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.
Instead of adding a markup for spacer, could we do a devenv-specific style?
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.
yes I've made that change now
This reverts commit b5c33c9.
- remove list-box__field tabindex
This reverts commit 927026b.
0e48f52
to
a5ffad8
Compare
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.
Thanks @emyarod for the update!
Hi there! 👋 If you're wondering why this issue was moved, we're currently updating our repo structure so that every package is found in the same project. This should not have any impact for you, but we wanted to give you a heads up in case you were wondering what is going on. If you have any questions, feel free to reach out to us on Slack or contact us at: [email protected]. Thanks! |
Closes #2240
This PR adds styles for the v10 combo box selection button and refactors multiselect selection button styles
Related:
https://github.com/IBM/carbon-components-react/issues/2109
https://github.com/IBM/carbon-components-react/issues/2113
Changelog
New
Changed
Testing / Reviewing