Skip to content

Feat: Add support for horizontal orientation to ListLayout delegate #8533

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nwidynski
Copy link
Contributor

@nwidynski nwidynski commented Jul 12, 2025

This PR adds support for horizontal orientations to ListLayout in preparation for a Carousel PR 👍 I can also try and expand the scope of this change to GridLayout, after getting the initial thoughts of the team. Currently I'm not entirely sure how a horizontal grid would be laid out (e.g. drop indicators horizontal or vertical?).

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

🧢 Your Project:

@nwidynski
Copy link
Contributor Author

@LFDanLu Would you be so kind to issue a build for this PR? I'm going insane because of a bug with virtualized dnd, which I can't seem to get rid of locally, even after reverting all changes and clearing caches. I would really like to know whether the issue exists on remote 😅


constructor(ref: RefObject<HTMLElement | null>) {
constructor(ref: RefObject<HTMLElement | null>, orientation?: Orientation) {
Copy link
Contributor Author

@nwidynski nwidynski Jul 12, 2025

Choose a reason for hiding this comment

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

Not entirely happy with having to pass this here, but I couldn't think of any reliable alternative. I guess we could do something similar to the drop target delegate and place this information in a data attribute, but is this really preferable?

Comment on lines -184 to -189
let keyboardDelegate = new ListKeyboardDelegate({
collection,
disabledKeys: selectionManager.disabledKeys,
disabledBehavior: selectionManager.disabledBehavior,
ref
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was there a reason for this delegate?

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