fix: prefer ancestor drop target over nearest-by-distance in DragManager#10170
Merged
snowystinger merged 2 commits intoJun 26, 2026
Merged
Conversation
snowystinger
reviewed
Jun 9, 2026
snowystinger
left a comment
snowystinger
left a comment
Member
There was a problem hiding this comment.
Looks good, reviewers will want to pull it down and copy the codesandbox into our storybook to verify as well.
I'll hopefully have some time to try it tomorrow.
snowystinger
approved these changes
Jun 9, 2026
snowystinger
left a comment
snowystinger
left a comment
Member
There was a problem hiding this comment.
Verified locally, thank you!
devongovett
approved these changes
Jun 26, 2026
Member
|
One thing to note is that this doesn't necessarily select the deepest ancestor in case there are multiple. But seems like an improvement. |
jsmitrah
added a commit
to jsmitrah/react-spectrum
that referenced
this pull request
Jun 29, 2026
fix: resolve lint and CI test compilation errors fix(table): floor fractional table width so columns don't overflow (adobe#10238) * fix(table): floor fractional table width so columns don't overflow calculateColumnSizes relies on cascadeRounding, which assumes the target column sizes sum to an integer. With a fractional table width (e.g. a percentage-based size) that invariant breaks and the rounded widths can exceed the available width by 1px, producing an unexpected horizontal scrollbar. Flooring the available width keeps the sum integral; integer widths are unchanged. Adds a regression test to TableUtils. Fixes adobe#9448. * Keep columns flush with a fractional table width Address review: instead of flooring the available width (which left a sub-pixel gap between the last column and the table edge), assign the leftover fraction to the last column inside cascadeRounding so the column widths sum exactly to the available width. Integer widths are unchanged. * Fix failing resize tests; keep integer widths whole The previous revision added the fractional remainder inside cascadeRounding using `fpTotal - intTotal`, but floating-point accumulation could leave a tiny residue on the last column even for integer table widths, breaking tests that assume whole-number widths (e.g. tableResizingTests). Revert cascadeRounding to its integer-only form and instead split the available width in calculateColumnSizes: distribute the floored (whole-pixel) width across the columns, then add the leftover fraction to the last column. Integer widths are now exactly whole numbers again, and a fractional width (e.g. 1000.5 -> [500, 500.5]) keeps the columns flush with the table edge. * Add test for fractional table widths with fp rounding error Covers a table width like 1000.7 where the leftover fraction lands on the last column. The columns still sum exactly to the available width and all but the last column are whole numbers; the last column is within one ULP of the expected fractional value. * use number string manipulation to make exact math --------- Co-authored-by: Rob Snow <rsnow@adobe.com> fix: shadcn registry build (adobe#10263) fix: make sure trackpad taps properly fire press events consistently (adobe#10225) * fix: make sure trackpad taps are not considered as virtual clicks * didnt save? fix(dialog): add aria-describedby support for alertdialog role (adobe#9924) * fix(dialog): add aria-describedby support for alertdialog role AlertDialog was rendering with role="alertdialog" but without aria-describedby, which the WAI-ARIA spec recommends for alertdialogs to reference the alert message content. Changes: - useDialog hook: generate a content ID via useSlotId when role is "alertdialog", return contentProps with the ID, and set aria-describedby on the dialog element - V3 Dialog: spread contentProps onto the content slot so the Content element receives the generated ID - RAC Dialog: destructure contentProps (available for user composition) The aria-describedby is only auto-wired for alertdialog role. Regular dialogs are unaffected. Users can override via the aria-describedby prop. Fixes adobe#9916 * fix: wire contentProps via TextContext for RAC alertdialog aria-describedby * test: add RAC integration test for alertdialog aria-describedby via Text slot * fix: sort TextContext import alphabetically in RAC Dialog * update tests, add warning to RAC * add describedby to s2 alert dialog * fix test * refactor, add tests, fix image labels in examples * fix lint * fix: allow aria-describedby override in v3 AlertDialog Extends SpectrumAlertDialogProps with AriaLabelingProps and passes {labelable: true} to filterDOMProps so user-supplied aria-describedby flows through to the inner Dialog instead of being stripped. Mirrors the S2 AlertDialog treatment already merged by snowystinger, and aligns v3 AlertDialog with regular v3 Dialog, which has these props transitively via AriaDialogProps. Adds a test verifying the override reaches the rendered alertdialog element. * fix(useDialog): drop contentId when aria-describedby is user-provided If the consumer supplies their own aria-describedby on an alertdialog, useDialog no longer generates a slot id for contentProps. This mirrors the existing handling of titleId vs aria-label and prevents downstream wrappers (e.g. RAC Dialog's TextContext for the description slot) from receiving an id no one references. Adds a test covering the override path. * fix(s2): wire aria-describedby on S2 AlertDialog via explicit contentId Generates a contentId in S2 AlertDialog and passes it to both the underlying Dialog (as aria-describedby) and to <Content /> (as id). This puts the aria-describedby target directly on the Content element rather than on the Text slot wrapper, and keeps the wiring contained within AlertDialog instead of relying on the RAC TextContext path. A consumer-supplied aria-describedby continues to take precedence; in that case we omit the generated id from <Content /> so we don't add an unused id. * fix(s2): import useId from react-aria/useId, not 'react' React.useId is React 18+ only, breaking test-16 and test-17 in CI. Every other S2 file uses the polyfilled useId from react-aria/useId. * fix some merge changes * move warning down to hook and remove extra code from s2 * test: use renamed DialogTester.getDialog() API after merge main renamed the DialogTester getters to methods (get dialog() -> getDialog()). The alertdialog aria-describedby test still used the old .dialog getter, which now returns undefined and fails the assertions. Match the getDialog() API already used elsewhere in this file. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * make more forgiving since spec says "should" --------- Co-authored-by: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Co-authored-by: Rob Snow <rsnow@adobe.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> fix: re-export RenderProps-related types from react-aria-components to (adobe#9955) fix downstream TS2742 The v1.17.0 types refactor split the bundled types.d.ts into per-source files. RenderProps extends StyleRenderProps extends DOMRenderProps, and StyleRenderProps['style'] uses StyleOrFunction — neither was publicly nameable. Downstream packages built with composite/declaration emit then fail with TS2742 on any inferred type whose expansion crosses these names. Adds `export` to StyleOrFunction and re-exports DOMRenderProps, PossibleLinkDOMRenderProps, DOMRenderFunction, ClassNameOrFunction, StyleOrFunction, and ChildrenOrFunction from the package entry. Type-only, additive — no runtime impact. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Robert Snow <rsnow@adobe.com> fix(useId): prevent FinalizationRegistry entry leak in useId (adobe#9853) * fix(useId): avoid FinalizationRegistry entry leak in useId * fix test in React 16/17 * add comments to test * additional test: changing the id should unregister the old token and register the new one --------- Co-authored-by: Robert Snow <rsnow@adobe.com> fix: prefer ancestor drop target over nearest-by-distance in DragManager (adobe#10170) Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Co-authored-by: Robert Snow <rsnow@adobe.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drag and drop now prefers an ancestor drop target over a closer-by-distance sibling, matching how nested drop zones are expected to resolve.
Why this matters
Reported in #10055:
findNearestDropTargetpicked the geometrically nearest target, so a small drop zone next to a containing one could win even when the pointer was inside the container. The fix prefers an ancestor target when the pointer is within it, before falling back to nearest-by-distance.Changes
DragManager's target selection to prefer an ancestor drop target containing the pointer.Testing
Added a DnD test covering the nested-target case.
Fixes #10055
AI was used for assistance.