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 for issue #5631 #5714

Merged
merged 1 commit into from Mar 2, 2015

Conversation

Projects
None yet
5 participants
@S-Ercan
Contributor

S-Ercan commented Mar 1, 2015

Fixes the flickering hovercard issue reported in #5631.

@S-Ercan S-Ercan referenced this pull request Mar 1, 2015

Closed

People's preview broken #5631

@AugierLe42e

This comment has been minimized.

Show comment
Hide comment
@AugierLe42e

AugierLe42e Mar 1, 2015

Contributor

Thx, this one annoyed me >.<

Contributor

AugierLe42e commented Mar 1, 2015

Thx, this one annoyed me >.<

@jhass

This comment has been minimized.

Show comment
Hide comment
@jhass

jhass Mar 1, 2015

Member

Sorry, but not an improvement in all cases for me: http://cloud.aeshna.de/u/mrzyx/hovercard.webm

I think it also changes look & feel quite a bit in the other places. Isn't there a way to debounce the additional events?

Member

jhass commented Mar 1, 2015

Sorry, but not an improvement in all cases for me: http://cloud.aeshna.de/u/mrzyx/hovercard.webm

I think it also changes look & feel quite a bit in the other places. Isn't there a way to debounce the additional events?

@S-Ercan

This comment has been minimized.

Show comment
Hide comment
@S-Ercan

S-Ercan Mar 1, 2015

Contributor

Ok, I've reverted the changes regarding hovercard positioning and used a different approach now to solve the flickering issue.

I've modified the _mouseleaveHandler logic so that, while mouseleave events can still be fired from both the parent and the hovercard, they are processed only if we have left both.
If you leave the hovercard but are still in the parent, or you leave the parent but are still in the hovercard, the handler returns false.
The absence of the first restriction was what caused the flickering problem.

Contributor

S-Ercan commented Mar 1, 2015

Ok, I've reverted the changes regarding hovercard positioning and used a different approach now to solve the flickering issue.

I've modified the _mouseleaveHandler logic so that, while mouseleave events can still be fired from both the parent and the hovercard, they are processed only if we have left both.
If you leave the hovercard but are still in the parent, or you leave the parent but are still in the hovercard, the handler returns false.
The absence of the first restriction was what caused the flickering problem.

@jhass jhass added this to the next-major milestone Mar 2, 2015

@jhass

This comment has been minimized.

Show comment
Hide comment
@jhass

jhass Mar 2, 2015

Member

Awesome this works great. Please just make sure to resolve the above comments and squash your commits into one:

git remote add upstream git://github.com/diaspora/diaspora.git
git fetch upstream
git checkout 5631-broken-people-preview
git rebase -i upstream/develop
# Choose pick for the first line, squash for the other ones, save & quit
git push -f origin 5631-broken-people-preview

As a tip for the future: You can edit a commit with git commit --amend, which is, given you're in a feature branch, is fine to do even after you pushed it.

Member

jhass commented Mar 2, 2015

Awesome this works great. Please just make sure to resolve the above comments and squash your commits into one:

git remote add upstream git://github.com/diaspora/diaspora.git
git fetch upstream
git checkout 5631-broken-people-preview
git rebase -i upstream/develop
# Choose pick for the first line, squash for the other ones, save & quit
git push -f origin 5631-broken-people-preview

As a tip for the future: You can edit a commit with git commit --amend, which is, given you're in a feature branch, is fine to do even after you pushed it.

Issue #5631. Made top and left coordinates of hovercards match those …
…of their parents.

Added line to changelog for bug fix of issue #5631.

Processing the 'mouseleave' event only if we've left both the parent and the hovercard.

Fixed indentation.

Replaced issue id with pull request id and fixed indentation.
@S-Ercan

This comment has been minimized.

Show comment
Hide comment
@S-Ercan

S-Ercan Mar 2, 2015

Contributor

Here goes! Indentation fixed, commits squashed.
Thanks for your feedback!

Contributor

S-Ercan commented Mar 2, 2015

Here goes! Indentation fixed, commits squashed.
Thanks for your feedback!

jhass added a commit that referenced this pull request Mar 2, 2015

@jhass jhass merged commit 15dd1b6 into diaspora:develop Mar 2, 2015

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
hound Hound has reviewed the changes.
@jhass

This comment has been minimized.

Show comment
Hide comment
@jhass

jhass Mar 2, 2015

Member

Thank you!

Member

jhass commented Mar 2, 2015

Thank you!

@Flaburgan

This comment has been minimized.

Show comment
Hide comment
@Flaburgan

Flaburgan Mar 2, 2015

Member

Thanks!

Member

Flaburgan commented Mar 2, 2015

Thanks!

@S-Ercan S-Ercan deleted the delftswa2014:5631-broken-people-preview branch Mar 2, 2015

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