feat(select): add wrapper and bottom shadow parts#30951
feat(select): add wrapper and bottom shadow parts#30951thetaPC wants to merge 2 commits intofeature-8.8from
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
This looks fine to me, the only minor concern I have is that there's no visual regression tests, but that's probably fine. No idea.
That makes sense, but @brandyscarney has made good points of why we shifted over to use this new approach to test parts. I just can't seem to remember what it was. Maybe less snapshots for us to maintain? |
brandyscarney
left a comment
brandyscarney
left a comment
There was a problem hiding this comment.
Looks good! Just requesting a name update on one.
| <label class="select-wrapper" id="select-label" onClick={this.onLabelClick} part="wrapper"> | ||
| {this.renderLabelContainer()} | ||
| <div class="select-wrapper-inner"> | ||
| <div class="select-wrapper-inner" part="wrapper-inner"> |
There was a problem hiding this comment.
This should be named inner to go with the way we are naming the inner wrappers in item components.
| })} | ||
| > | ||
| <label class="select-wrapper" id="select-label" onClick={this.onLabelClick}> | ||
| <label class="select-wrapper" id="select-label" onClick={this.onLabelClick} part="wrapper"> |
There was a problem hiding this comment.
This is likely fine, let's just make sure we all agree on this name.
| * @part error-text - Supporting text displayed beneath the select when the select is invalid and touched. | ||
| * @part bottom - The container element for helper text, error text, and counter. | ||
| * @part wrapper - The clickable label element that wraps the entire form field (label text, slots, selected values or placeholder, and toggle icons). | ||
| * @part wrapper-inner - The inner element of the wrapper that does not include the label text. |
There was a problem hiding this comment.
Can we change this to define what it wraps instead of what it doesn't?
|
Sorry I missed your previous comments. We don't need screenshot tests here since we're defining the colors ourselves. If these were the default colors for the elements we would verify them visually to ensure they look correct because the defaults may change frequently. Because we're defining what color the token should use we can just verify that the right element is styled by it. |
3 participants
Issue number: resolves #29918
What is the current behavior?
Developers can't customize elements within select like the bottom container because the component is a shadow component.
What is the new behavior?
bottom,wrapper, andwrapper-innerpartsDoes this introduce a breaking change?
Other information
N/A