Skip to content

feat: add AttachmentGrid component - #10561

Open
DPandyan wants to merge 3 commits into
mainfrom
attachmentgrid
Open

feat: add AttachmentGrid component#10561
DPandyan wants to merge 3 commits into
mainfrom
attachmentgrid

Conversation

@DPandyan

@DPandyan DPandyan commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Closes

✅ 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
  • I understand every change in this PR and can explain why it's there.
  • If AI-assisted, I followed our AI contribution guidance and pointed my assistant at CLAUDE.md.

📝 Test Instructions:

🧢 Your Project:

Adobe

@DPandyan DPandyan changed the title add: AttachmentGrid component feat: add AttachmentGrid component Sep 3, 2026
@rspbot

rspbot commented Sep 3, 2026

Copy link
Copy Markdown

@rspbot

rspbot commented Sep 3, 2026

Copy link
Copy Markdown

...focusRing()
});

const gridGap = css('gap: 6px;');

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I can swap this out for 8px, Figma had 6, wasn't sure.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can use custom values in a style macro like so
gap: '[6px]'
or if it needs to respond to font size or scaling, you can make use of size() or space()
https://react-spectrum.adobe.com/style-macro#space

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it really a static 6px at all sizes? Feels like the scale should change the gap size

)
};

export const WithAttachmentGrid: Story = {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thought this would be a useful story, can remove.

export const Overflow: Story = {
name: 'Overflow (vertical scroll fade)',
render: args => (
<div style={{width: 320, resize: 'horizontal', overflow: 'hidden'}}>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

maybe this needs padding

@LFDanLu LFDanLu added the release label Sep 3, 2026
@rspbot

rspbot commented Sep 3, 2026

Copy link
Copy Markdown

@rspbot

rspbot commented Sep 3, 2026

Copy link
Copy Markdown
## API Changes

@react-spectrum/ai

/@react-spectrum/ai:ExecutionTraceItem

 ExecutionTraceItem {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
-  children: string
+  children: ReactNode
   detail?: ReactNode
   detailMaxHeight?: number = 120
   icon?: ReactNode
   id?: string
   styles?: StyleString
 }

/@react-spectrum/ai:ResponseStatusTitle

 ResponseStatusTitle {
-  children: string
+  children: React.ReactNode
   id?: string
   level?: number = 3
   pixelLoader?: Array<Cell> | Array<Array<Cell>>
   styles?: StyleString

/@react-spectrum/ai:ExecutionTraceItemProps

 ExecutionTraceItemProps {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
-  children: string
+  children: ReactNode
   detail?: ReactNode
   detailMaxHeight?: number = 120
   icon?: ReactNode
   id?: string
   styles?: StyleString
 }

/@react-spectrum/ai:ResponseStatusTitleProps

 ResponseStatusTitleProps {
-  children: string
+  children: React.ReactNode
   id?: string
   level?: number = 3
   pixelLoader?: Array<Cell> | Array<Array<Cell>>
   styles?: StyleString

/@react-spectrum/ai:AttachmentGrid

+AttachmentGrid <T> {
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children?: ReactNode | (T) => ReactNode
+  dependencies?: ReadonlyArray<any>
+  id?: string
+  items?: Iterable<T>
+  styles?: StyleString
+}

/@react-spectrum/ai:AttachmentGridItem

+AttachmentGridItem {
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children: ReactNode | (AttachmentRenderProps) => ReactNode
+  id?: Key
+  isInvalid?: boolean
+  size?: 'XS' | 'S' | 'M' | 'L' | 'XL'
+  styles?: StyleString
+  textValue?: string
+  uploadProgress?: number
+}

/@react-spectrum/ai:AttachmentGridProps

+AttachmentGridProps <T> {
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children?: ReactNode | (T) => ReactNode
+  dependencies?: ReadonlyArray<any>
+  id?: string
+  items?: Iterable<T>
+  styles?: StyleString
+}

/@react-spectrum/ai:AttachmentGridItemProps

+AttachmentGridItemProps {
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  children: ReactNode | (AttachmentRenderProps) => ReactNode
+  id?: Key
+  isInvalid?: boolean
+  size?: 'XS' | 'S' | 'M' | 'L' | 'XL'
+  styles?: StyleString
+  textValue?: string
+  uploadProgress?: number
+}

@rspbot

rspbot commented Sep 3, 2026

Copy link
Copy Markdown

Agent Skills Changes

Modified (9)
Install

React Spectrum S2:

npx skills add https://d1pzu54gtk2aed.cloudfront.net/pr/a6370f063a1c0cbe033bddc0ae8ad9254063b462/

React Aria:

npx skills add https://d5iwopk28bdhl.cloudfront.net/pr/a6370f063a1c0cbe033bddc0ae8ad9254063b462/

@DPandyan
DPandyan marked this pull request as ready for review September 3, 2026 17:55
...focusRing()
});

const gridGap = css('gap: 6px;');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is it really a static 6px at all sizes? Feels like the scale should change the gap size

let domRef = useDOMRef(ref);

return (
<ListBox

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should the container have a focus ring? Or are the items focusable its hard to tell

const hasContent = ':has([data-slot=content])';

const gridStyles = style({
display: 'grid',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can there be a mix of cards that are thumbnails and cards that have the description?

const gridStyles = style({
display: 'grid',
gridTemplateColumns: {
default: 'repeat(auto-fill, minmax(64px, 1fr))',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the container is wide and there aren't enough thumbs to fill it in the first row, i think they'll end up spaced apart
I think you just want

default: 'repeat(auto-fill, minmax(64px, auto))'

though that only matters if the attachments can be varying sizes, if they can't, then you could just do

default: 'repeat(auto-fill, 64px)'

size?: 'XS' | 'S' | 'M' | 'L' | 'XL';
/** Whether the attachment has an error. */
isInvalid?: boolean;
uploadProgress?: number;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

missing description

Comment on lines +140 to +142
aria-label={ariaLabel}
aria-labelledby={ariaLabelledby}
aria-describedby={ariaDescribedby}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these and id are all just getting passed straight through, we can use filterDOMProps with labeling set to true
then we can just spread the result of that

@LFDanLu LFDanLu removed the release label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants