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

Keyboard drag dimension clipping fix #137

Merged
merged 3 commits into from
Oct 17, 2017

Conversation

jaredcrowe
Copy link
Contributor

@jaredcrowe jaredcrowe commented Oct 10, 2017

fixes #136

// Get the true visible bounds of the droppables.
// We calculate it once here and pass it on in an
// object along with the original droppable.
.map((droppable: DroppableDimension): Candidate => ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

basically we just use the droppable's visible bounds everywhere now instead of the droppable's dimension. we calculate it once here and pass it along with the droppable.

{ x: fragment.right, y: fragment.bottom },
];
};
const getCorners = (bounds: Spacing): Position[] => [
Copy link
Collaborator

Choose a reason for hiding this comment

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

awesome! maybe this should go into spacing.js

@@ -31,70 +28,81 @@ type GetBestDroppableArgs = {|
droppables: DroppableDimensionMap,
|}

type Candidate = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

whoa - can you explain this a little?

@jaredcrowe jaredcrowe changed the base branch from droppable-dimension-includes-placeholder to master October 12, 2017 01:33
@jaredcrowe jaredcrowe force-pushed the keyboard-drag-dimension-clipping-fix branch from 3975e02 to 3713f89 Compare October 12, 2017 02:16
@jaredcrowe
Copy link
Contributor Author

@alexreardon I've re-pointed this at master and actioned your previous comments. lmk if you have a chance to review this, otherwise I'll get someone in AK to take a look.

Basically the difference is that we're now using the clipped bounds rather than the full droppable dimension in get-best-cross-axis-droppable.

@alexreardon
Copy link
Collaborator

alexreardon commented Oct 12, 2017 via email

return a.page.withMargin[axis.start] - b.page.withMargin[axis.start];
})[0];
return a[axis.start] - b[axis.start];
})[0].droppable;
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a chance that candidates could be an empty array?

edit: sorry just saw the candidates.length check above, it was hidden in the PR folded lines

@jaredcrowe jaredcrowe merged commit b722603 into master Oct 17, 2017
@alexreardon alexreardon deleted the keyboard-drag-dimension-clipping-fix branch November 7, 2017 11:09
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.

Transitioning between lists with the keyboard does not respect container clipping
3 participants