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.

Sign up for GitHub

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

Jump to bottom

Fixed #8710 - Position: collision: "flip" doesn't work for my: "right top", at: "right bottom" (in some situations) #1071

Closed
wants to merge 4 commits into from
Closed

Fixed #8710 - Position: collision: "flip" doesn't work for my: "right top", at: "right bottom" (in some situations) #1071

wants to merge 4 commits into from

Conversation

meyertee
Copy link
Contributor

@meyertee meyertee commented Sep 4, 2013

Bug #8719

Reverted this commit: 7f808b2

The intention of the commit is unclear.. I added unit tests to show that the code before that commit already did what it aimed to fix.

http://bugs.jqueryui.com/ticket/8710

@meyertee
Revert "Position: Check to also make sure to only flip if more of the…
d4a21ea 
… element is inside "within""

This reverts commit 7f808b2.

Conflicts:
	ui/jquery.ui.position.js
@meyertee
added unit tests for bug 8710
beab2ac
@lazd
Copy link

lazd commented Nov 18, 2014

What's the status on this PR? @meyertee, do we know if the problem is present in the latest release of jQuery UI?

@meyertee
Copy link
Contributor Author

@lazd according to this fiddle the issue still exists: http://jsfiddle.net/meyertee/twrLf/
It uses jqueryui 1.11.2. Drag the vertical divider above the Result panel vertically to see the red layer jump as it gets smaller..

@lazd
Copy link

lazd commented Nov 18, 2014

@meyertee thanks for following up. I wonder if we can get this PR back in shape and push for a merge? At Adobe, we're patching jQuery UI to fix this and have been doing so for over a year.

@jzaefferer
Copy link
Member

This merges cleanly into master, no merge conflicts. I verified that the new test fails without the changes. Build passes, no jshint or jscs errors. Including the patched file in the fiddle provided in the ticket shows that the issue is fixed: http://jsfiddle.net/u697pzoq/

There are a few comments in the test file that should be uppercased, but that's the only issue I've found. @scottgonzalez anything else?

@meyertee
Copy link
Contributor Author

Let me know if there are any other issues, I'm happy to fix them.

@scottgonzalez
Copy link
Member

Well, since the original commit didn't reference a bug or include tests, I guess we'll just go with this and see if anyone reports bugs based on within :-P

Thanks for handling this @meyertee. Sorry it took so long for us to get to.

scottgonzalez pushed a commit that referenced this pull request Jan 9, 2015
@meyertee @scottgonzalez
Position: Restore old flip collision handling
ebaaca7 
This reverts commit 7f808b2.

Fixes #8710
Ref gh-1071
@lazd
Copy link

lazd commented Jan 9, 2015

Hooray!

@meyertee
Copy link
Contributor Author

Great, thank you for merging it :)

scottgonzalez pushed a commit that referenced this pull request Feb 9, 2015
@meyertee @scottgonzalez
Position: Restore old flip collision handling
276cd5c 
This reverts commit 7f808b2.

Fixes #8710
Ref gh-1071
(cherry picked from commit ebaaca7)
scottgonzalez pushed a commit that referenced this pull request Feb 9, 2015
@meyertee @scottgonzalez
Position: Add unit tests for bug 8710
9db4057 
Ref #8710
Closes gh-1071
(cherry picked from commit 4de983c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@meyertee @lazd @jzaefferer @scottgonzalez