feat: allow custom react element for S2 Picker value#9541
feat: allow custom react element for S2 Picker value#9541LFDanLu merged 12 commits intoadobe:mainfrom
Conversation
| * Custom renderer for the selected value shown in the button. | ||
| * Matches the `SelectValue` children render type. | ||
| */ | ||
| renderValue?: SelectValueProps<T>['children'] |
There was a problem hiding this comment.
I think we should just pass in the selected items (i.e. (selectedItems: T[]) => ReactNode) here, and only call it when there is at least one item selected. We already have the placeholder prop to customize the placeholder text, and you can omit renderValue if you want the default text. Is that sufficient for your use case or did you need one of the other render props?
There was a problem hiding this comment.
Yes, it's enough for our use case -- was probably being too proactive trying to think of a general situation.
BrknRules
force-pushed
the
S2-Picker-renderValue
branch
from
0645caf to
7f8c3fd
Compare
BrknRules
force-pushed
the
S2-Picker-renderValue
branch
from
7f8c3fd to
d5c4f3c
Compare
BrknRules
left a comment
BrknRules
left a comment
There was a problem hiding this comment.
Some notes on implementation choices -- would appreciate thoughts!
| display: 'flex', | ||
| alignItems: 'center' | ||
| alignItems: 'center', | ||
| height: '100%' |
There was a problem hiding this comment.
Locked height to picker button height. Enough for our use case as we don't have anything larger than the button that should cause it to resize.
devongovett
left a comment
devongovett
left a comment
There was a problem hiding this comment.
Looks good. Couple lint/ts issues.
BrknRules
force-pushed
the
S2-Picker-renderValue
branch
from
58c16a4 to
7aeefae
Compare
github-project-automation
bot
moved this from 🩺 To Triage
to ✅ Done
in RSP Component Milestones
github-project-automation
bot
moved this from ✅ Done
to ✏️ To Groom
in RSP Component Milestones
github-project-automation
bot
moved this from ✏️ To Groom
to ✅ Done
in RSP Component Milestones
github-project-automation
bot
moved this from ✅ Done
to ✏️ To Groom
in RSP Component Milestones
snowystinger
changed the title
There was a problem hiding this comment.
To note for testing:
In Chrome VO, there seems to be an issue where it thinks there is an interactive element inside the docs example's button. There is not, and the associated warning does not appear in Safari
Chromatic green: https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=1116
reidbarber
left a comment
reidbarber
left a comment
There was a problem hiding this comment.
Looks good! Nothing blocking
| const renderedValue = selectedItems.length > 0 && renderValue | ||
| ? renderValue(selectedValues) | ||
| : defaultRenderedValue; |
There was a problem hiding this comment.
This should probably be selectedValues.length > 0 to prevent the edge case if you had all null items. Super edge case though, I don't even think null items is something we'd care to support.
| <div style={{display: 'flex', gap: 4, height: '80%'}}> | ||
| {selectedItems.map(item => ( | ||
| <img key={item.id} src={item.icon} alt={item.label} /> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
Good example, we could probably use AvatarGroup here.
| <div className={style({ display: 'flex', gap: 4, height: '80%' })}> | ||
| {selectedItems.map(item => ( | ||
| <Avatar slot={null} key={item.id} src={item.avatar} alt={item.name} /> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
Same, maybe AvatarGroup would better since it's an established pattern.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-project-automation
bot
moved this from 👀 In Review
to ✅ Done
in RSP Component Milestones
6 participants
This adds a
renderValueprop to Picker S2, which allows users to render custom elements within the Picker's select based off of selected items.