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

Ease droppable orientation restrictions #1907

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MaximSagan
Copy link

The problem

As documented in #1906, a draggable currently cannot be moved by keyboard to another droppable if the droppables are oriented in the same direction as the draggables).

Internally, this was related to the behavior in src/state/move-in-direction/index.js where moveToNextPlace() (move internally) would be called if the arrow key is in the same axis as the draggable's direction (isMovingOnMainAxis === true), and moveCrossAxis() (which allows movement between droppables) would be called if the arrow key is on the other axis (isMovingOnMainAxis === false).

The problem here (and it is clear from the function name moveCrossAxis) is that it pre-supposes that droppables must be oriented along the cross axis to the main axis that the draggables are oriented along. This is overly restrictive, especially when we consider vertical lists where the number of draggables and droppables are both dynamic and it is unrealistic to place droppables horizontally side-by-side without undesirable horizontal scrolling.

What I've done

I have naively renamed moveCrossAxis to moveAxis (this involved renaming a directory). It is now also being called when isMovingOnMainAxis === true if moveToNextPlace yielded a null result (i.e. there was no internal position that the draggable could be moved to, which should indicate that the first/last position within the droppable has been reached and the user is trying to reach another droppable).

I have added the isMovingOnMainAxis boolean as an option for moveAxis, and it is being passed to getBestAxisDroppable (formerly getBestCrossAxisDroppable) where the majority of the changes have been made (to support both axes).

As this represents a change in functionality (purely additive in my opinion), I have updated the docs as well.

Note

I have not added any unit tests for this for this new functionality. As I do not expect this PR to be approved and merged in its current form, I thought I should wait before adding any so as not to waste time.

@atlassian-cla-bot
Copy link

atlassian-cla-bot bot commented Aug 3, 2020

Hooray! All contributors have signed the CLA.

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