Skip to content

fix: use min-width so Firefox respects applied column widths#9871

Merged
LFDanLu merged 5 commits intomainfrom
9865
Apr 3, 2026
Merged

fix: use min-width so Firefox respects applied column widths#9871
LFDanLu merged 5 commits intomainfrom
9865

Conversation

@LFDanLu
Copy link
Copy Markdown
Member

@LFDanLu LFDanLu commented Apr 1, 2026

Closes #9865

✅ 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:

Go to the "fixed column width" and "table example" RAC table stories. The fixed column width table should render each of its columns with 100px of width and not fill its wrapping container. Shrinking the width of the "Action" column in the "table example" story should not make the other columns expand in size.

🧢 Your Project:

RSP

the table would also increase the rest of the column widths when trying to shrink a single column in some cases too
@github-actions github-actions bot added the RAC label Apr 1, 2026
@rspbot
Copy link
Copy Markdown

rspbot commented Apr 1, 2026

@LFDanLu
Copy link
Copy Markdown
Member Author

LFDanLu commented Apr 1, 2026

yihuiliao
yihuiliao previously approved these changes Apr 1, 2026
...style,
tableLayout: 'fixed',
width: 'fit-content'
width: 'min-content'
Copy link
Copy Markdown
Contributor

@nwidynski nwidynski Apr 2, 2026

Choose a reason for hiding this comment

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

Could you add a code comment pointing to the upstream issue? We may want to revisit this once it gets resolved. Also, for future reference, the reason why I didn't suggest the "extended" version of fit-content, e.g. min(max-content, max(min-content, stretch)), is because max-content is actually the value causing the issue - not fit-content itself, which is basically an alias for the above.

@rspbot
Copy link
Copy Markdown

rspbot commented Apr 2, 2026

yihuiliao
yihuiliao previously approved these changes Apr 2, 2026
...style,
tableLayout: 'fixed',
width: 'fit-content'
// due to https://bugzilla.mozilla.org/show_bug.cgi?id=1959353, we can't use "fit-content".
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 we adde a chromatic story as a test against this? we run against FF there, so should be reproducible

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added the RAC story to the S2 chromatic since we don't have a RAC chromatic. Would've just added a S2 story but our S2 table doesn't actually exhibit the original issue.

@rspbot
Copy link
Copy Markdown

rspbot commented Apr 2, 2026

@LFDanLu
Copy link
Copy Markdown
Member Author

LFDanLu commented Apr 3, 2026

@snowystinger
https://www.chromatic.com/build?appId=5f0dd5ad2b5fc10022a2e320&number=1166 with old fit-content (not sure why it detected changes for the other S2 table stories)
https://www.chromatic.com/test?appId=5f0dd5ad2b5fc10022a2e320&id=69cf09ade1014b389adf95a4
with the fix

@rspbot
Copy link
Copy Markdown

rspbot commented Apr 3, 2026

@rspbot
Copy link
Copy Markdown

rspbot commented Apr 3, 2026

## API Changes

@react-spectrum/s2

/@react-spectrum/s2:ActionMenu

 ActionMenu <T extends {}> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   align?: 'start' | 'end' = 'start'
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean
   children: ReactNode | (T) => ReactNode
   defaultOpen?: boolean
   direction?: 'bottom' | 'top' | 'left' | 'right' | 'start' | 'end' = 'bottom'
   disabledKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
   isOpen?: boolean
   isQuiet?: boolean
   items?: Iterable<T>
   menuSize?: 'S' | 'M' | 'L' | 'XL' = 'M'
   onAction?: (Key) => void
   onOpenChange?: (boolean) => void
-  shouldCloseOnSelect?: boolean
   shouldFlip?: boolean = true
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   styles?: StylesProp
 }

/@react-spectrum/s2:ActionMenuProps

 ActionMenuProps <T> {
   UNSAFE_className?: UnsafeClassName
   UNSAFE_style?: CSSProperties
   align?: 'start' | 'end' = 'start'
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean
   children: ReactNode | (T) => ReactNode
   defaultOpen?: boolean
   direction?: 'bottom' | 'top' | 'left' | 'right' | 'start' | 'end' = 'bottom'
   disabledKeys?: Iterable<Key>
   id?: string
   isDisabled?: boolean
   isOpen?: boolean
   isQuiet?: boolean
   items?: Iterable<T>
   menuSize?: 'S' | 'M' | 'L' | 'XL' = 'M'
   onAction?: (Key) => void
   onOpenChange?: (boolean) => void
-  shouldCloseOnSelect?: boolean
   shouldFlip?: boolean = true
   size?: 'XS' | 'S' | 'M' | 'L' | 'XL' = 'M'
   styles?: StylesProp
 }

@LFDanLu LFDanLu added this pull request to the merge queue Apr 3, 2026
Merged via the queue into main with commit 139fe90 Apr 3, 2026
31 checks passed
@LFDanLu LFDanLu deleted the 9865 branch April 3, 2026 18:16
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.

ResizableTableContainer: column widths not respected in Firefox due to table-layout: fixed + width: fit-content

6 participants