Skip to content

Conversation

@dkario
Copy link
Contributor

@dkario dkario commented Sep 25, 2025

For the new avatar style macro exported from Menu, I copied a subset of ComboBox's image style macro.

Avatar controls size via its size prop, so I didn't include that in the style macro.

image

Closes: N/A

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

  • In new "With Avatars" Combobox and Picker stories, verify avatars are positioned and sized correctly

🧢 Your Project:

Adobe

Dylan Kario added 2 commits September 24, 2025 17:04
For the new `avatar` style macro exported from Menu, I copied a subset the `image` style macro.

Avatar controls size via its `size` prop, so I didn't include that in the style macro.
Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like the right direction to go

objectFit: 'contain'
});

export let avatar = style({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure we're going to have avatar support in a menu, best to define this over in ComboBox and Picker as needed

@dkario dkario changed the title feat: add Avatar support for ComboBoxItem feat: add Avatar support for ComboBoxItem and PickerItem Sep 25, 2025
Comment on lines 86 to 107
```tsx render
"use client";
import {Avatar, Picker, PickerItem, Text} from '@react-spectrum/s2';

const users = Array.from({length: 10}, (_, i) => ({
name: `User ${i + 1}`,
email: `user${i + 1}@example.com`,
avatar: `https://i.imgur.com/kJOwAdv.png`
}));

<Picker label="Share" items={users}>
{(item) => (
<PickerItem textValue={item.name}>
{/*- begin highlight -*/}
<Avatar slot="avatar" src={item.avatar} />
{/*- end highlight -*/}
<Text slot="label">{item.name}</Text>
<Text slot="description">{item.email}</Text>
</PickerItem>
)}
</Picker>
```
Copy link
Contributor Author

@dkario dkario Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this doc example is good enough for the site, or if it's okay to have two tsx render blocks one after another like I did here. Same thoughts for the ComboBox docs I added

Comment on lines +617 to +620
context={AvatarContext}
value={{slots: {
avatar: {size: avatarSize[size], styles: avatar}
}}}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the right approach to add another child DefaultProvider here?

width: 'full'
});

const avatar = style({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The style macro variables in Picker.tsx and ComboBox.tsx are named a bit inconsistently, do you prefer this to be called avatar or avatarStyle(s)?

@dkario dkario marked this pull request as ready for review September 25, 2025 19:42
@dkario
Copy link
Contributor Author

dkario commented Sep 25, 2025

Closing, reopened as non-fork PR: #8931

@dkario dkario closed this Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants