-
Notifications
You must be signed in to change notification settings - Fork 87
Update Select
for SLDS2
#468
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
base: support-slds-2
Are you sure you want to change the base?
Conversation
reg-suit detected visual differences. Check this report, and review them. 🔴🔴🔴🔴🔴🔴🔴🔴 🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵 What do the circles mean?The number of circles represent the number of changed images.🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items How can I change the check status?If reviewers approve this PR, the reg context status will be green automatically. |
9f261af
to
3b03dcc
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.
Pull Request Overview
This PR updates the Select
component to align with SLDS2 guidelines by passing id
as htmlFor
and inserting the .slds-select_container
wrapper for single-select elements.
- Switched
FormElement
prop fromid
tohtmlFor
- Wrapped non-multiple
<select>
in a<div className='slds-select_container'>
Comments suppressed due to low confidence (1)
src/scripts/Select.tsx:81
- Consider adding or updating unit tests to cover both rendering paths (with
.slds-select_container
for single-select and the direct<select>
for multiple).
<div className='slds-select_container'>{selectElem}</div>
}; | ||
return ( | ||
<FormElement {...formElemProps}> | ||
{rprops.multiple ? ( |
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.
[nitpick] Destructuring multiple
from props (e.g., const { multiple } = props
) and using it directly can improve readability instead of rprops.multiple
.
{rprops.multiple ? ( | |
{multiple ? ( |
Copilot uses AI. Check for mistakes.
3b03dcc
to
da98ae1
Compare
What I did
id
ascontrolId
.slds-select_container
inside theFormElement
Reference
https://v1.lightningdesignsystem.com/components/select/