Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2024 12 #65

Merged
merged 17 commits into from
Dec 13, 2024
Merged

2024 12 #65

merged 17 commits into from
Dec 13, 2024

Conversation

alirezamirian
Copy link
Owner

No description provided.

@alirezamirian alirezamirian force-pushed the 2024-12 branch 2 times, most recently from 7fc3a51 to 9fffa35 Compare December 8, 2024 17:13
@alirezamirian alirezamirian force-pushed the 2024-12 branch 3 times, most recently from 3698cf4 to 282c59f Compare December 9, 2024 21:16
…d the original one

One of the addressed issues is adobe/react-spectrum#1842 which is [fixed](adobe/react-spectrum#1843) already.

 The other issue is adobe/react-spectrum#4391 which is possible to work around in a wrapper.
Since @react-aria/[email protected] getKeyLeftOf and getKeyRightOf is removed if orientation is vertical and layout is "stack".

Using the single-argument overload of the base constructor doesn't set a default value for orientation (which seems a little random), which in turn makes the if statement that removes the methods not run.

https://github.com/adobe/react-spectrum/blob/8228e4efd9be99973058a1f90fc7f7377e673f78/packages/%40react-aria/selection/src/ListKeyboardDelegate.ts#L67
if `title` is set on `Item`, the default getItemCollection method assumes the item's children contain child nodes, and sets hasChildNodes to `true`.
…collection component

useCollectionSearchInput had this advantage of allowing for connecting an input to ANY collection without requiring support from the collection component. However the collection component needed to support selectionManagerRef, so there was still a need to make the collection component compatible with useCollectionSearchInput. Plus, the usage API isn't simple enough, as it requires:
- creating a ref to hold selection manager. Passing that ref to selectionManagerRef, and importing the right type for the ref value.
- importing and invoking useCollectionSearchInput, and applying the returned props on the connected input.

That's a lot of not-so-intuitive work.
Considering any supported collection component still needs to support `CollectionRefProps` and utilize `useCollectionRef` to support connected input, the implementation is changed to have the event listeners baked into the collections (via a new useCollectionFocusProxy hook). This way at least the usage API is as simple as passing a ref to the connected input to `focusProxyRef` prop of the collection component.
Copy link

cypress bot commented Dec 12, 2024

JUI    Run #259

Run Properties:  status check passed Passed #259  •  git commit 62436d4382 ℹ️: Merge b3123b75d1d970bf1738acd5db8c14fd47c16bbc into 6de51abcc0a711bdff6155cddc57...
Project JUI
Branch Review 2024-12
Run status status check passed Passed #259
Run duration 02m 47s
Commit git commit 62436d4382 ℹ️: Merge b3123b75d1d970bf1738acd5db8c14fd47c16bbc into 6de51abcc0a711bdff6155cddc57...
Committer Alireza Mirian
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 7
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 340
View all changes introduced in this branch ↗︎

Trying a little wait between finding the menu in the dom and scrolling, just in case the scroll handlers inside menu overlay are set up with a tiny delay. Doesn't seem that plausible though.
For some unknown reason, scroll events were triggered upon mouse wheel, even though scroll
was prevented. It didn't happen when running locally headless with `cypress run`, so it seems like the only differentiating factor is the OS being linux in the CI env.

The test is changed to compare actual scrollY of the window, instead of checking if scroll event handlers are called.
@alirezamirian alirezamirian merged commit 594d1fe into master Dec 13, 2024
21 checks passed
@alirezamirian alirezamirian deleted the 2024-12 branch December 13, 2024 22:50
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.

1 participant