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

Fix dock dragging and dropping #16863

Merged
merged 2 commits into from Mar 5, 2018

Conversation

Projects
None yet
4 participants
@matthewwithanm
Member

matthewwithanm commented Mar 1, 2018

This fixes #16769, which described an issue whereby trying to drop into
a closed dock would cause the dock to repeatedly open and close.

I tried to describe in comments why this was happening and how we can
make sure to avoid it.

The solution adds another DOM measurement which I'm not too thrilled
about but I profiled and it didn't seem to be an issue.

Fix dock dragging and dropping
This fixes #16769, which described an issue whereby trying to drop into
a closed dock would cause the dock to repeatedly open and close.

I tried to describe in comments why this was happening and how we can
make sure to avoid it.

The solution adds another DOM measurement which I'm not too thrilled
about but I profiled and it didn't seem to be an issue.

@matthewwithanm matthewwithanm requested review from daviwil and maxbrunsfeld Mar 1, 2018

@daviwil daviwil requested a review from as-cii Mar 1, 2018

@as-cii

as-cii approved these changes Mar 1, 2018

Looks good @matthewwithanm! Measuring one more time shouldn't be a huge concern because we were already calling getBoundingClientRect inside pointWithinHoverArea, so unless I am missing something this pull request won't add extra reflows.

Thanks for jumping on this, please merge at will once tests are green again. ⚡️

@daviwil

daviwil approved these changes Mar 1, 2018

Thanks a lot Matthew!

Make sure there's a single source of truth for dock hover state
Previously, the workspace's idea of the hovered dock and the docks'
themselves could be out of sync.

@matthewwithanm matthewwithanm merged commit e3a464e into master Mar 5, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

maxbrunsfeld added a commit that referenced this pull request Mar 5, 2018

@jasonrudolph jasonrudolph deleted the fb-mdt-fix-dock-dragndrop branch Apr 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment