feat: xs select component#22036
Conversation
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for v11-carbon-web-components ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22036 +/- ##
==========================================
- Coverage 95.09% 95.07% -0.03%
==========================================
Files 541 541
Lines 45179 45179
Branches 6544 6510 -34
==========================================
- Hits 42965 42954 -11
- Misses 2085 2096 +11
Partials 129 129
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
emyarod
left a comment
emyarod
left a comment
There was a problem hiding this comment.
looks good to me! just needs some snapshot updates to pass CI
alina-jacob
left a comment
alina-jacob
left a comment
There was a problem hiding this comment.
Hi @riddhybansal, thanks for working on this!
The WC version is good to go! 🚀
Noting down 3 bugs in the React version
- The overall field input needs to be centre aligned to that of the label and the helper text. Currently, the React version looks like its top aligned. The WC version is implemented correctly.
- The invalid and error states are showing up incorrectly for the Inline styling. This could also be because the component is fundamentally buggy, not your PR's doing.
- FYI: While reviewing @maradwan26's PR on XS Text Input i could find other related PRs which may be the root cause. PR in reference, Issue opened to resolve the same.
- The Inline standalone story, for Invalid state's helper text throws a rendering error. Again, seems like a fundamental issue and not your PR's doing.
I think for this PR to be visually approved and keep it at a manageable scope, pointer number 1 needs to be resolved. The other pointers need to be triaged before resolving.
alina-jacob
left a comment
alina-jacob
left a comment
There was a problem hiding this comment.
Revisiting pointer number 1 of this comment, looks like its an overall styling issue with the Inline variant which needs to be resolved and addressed with this issue.
Rest is good to go! 🚀
sangeethababu9223
added
status: ready to merge 🎉
and removed
status: one more review 👀
labels
riddhybansal
force-pushed
the
select_component
branch
from
a9a946e to
917b3df
Compare
riddhybansal
force-pushed
the
select_component
branch
from
cb2ffd0 to
447d7f2
Compare
cdca471
4 participants
Closes #21759
Closes #22031
This PR adds xs size for select component for both react and web components with contextual layout tokens.
Changelog
New
Changed
Removed
Testing / Reviewing
Verify all sizes of select component for both react and web-components are as per spec.
NOTE: A WithLayoutContext story is added to test if the layout context system is adopted properly. It will be deleted before merging.
PR Checklist
As the author of this PR, before marking ready for review, confirm you:
Updated documentation and storybook examplesWrote passing tests that cover this changeMore details can be found in the pull request guide