Dialog & Resizable styles fixed NE,SE,S,E handles. Fixed #9521 ui.Dialog... #1092

Closed
wants to merge 1 commit into
from

Projects

None yet

6 participants

@dekajp
dekajp commented Sep 28, 2013

...:Resizable dialogs move/resize out of viewport boundary - which results in scrollbar

@jzaefferer
Member

Before I mess up the author information again, can you please rebase this PR on top of latest origin/master and change your git email address to the correct one? Thank you.

@dekajp
dekajp commented Oct 18, 2013

@jzaefferer i will rebase all my pending PRs , also i see some formatting issues .

@dekajp
dekajp commented Oct 20, 2013

Please hold on this PR , looks like moveToTop is failing in regression

@kborchers
Member

@dekajp any progress on getting this reviewable?

@dekajp
dekajp commented Nov 10, 2013

@kborchers i think this PR can move forward. looks like moveToTop dialog: methods: moveToTop: content scroll stays intact is still failing in Chrome , where as it works in FF in my machine (mac os) . i updated my comment on this commit - e263ebd

sorry for delay

@jzaefferer
Member

We've landed a few dialog fixes in master in the last two days, including the issue you commented on. If you've seen test failures unrelated to your patch (especially in IE8), rebasing should help.

@dekajp
dekajp commented Nov 16, 2013

@jzaefferer 2 tests are still failing on master branch but only for chrome-30 , it works in FF25 in MacOS. one dialog: options: resizable and dialog: options: #4826: setting resizable false toggles resizable on dialog . i think these are unrelated.

on side note - i am executing these unit test cases in browser. my unit test case checks for scrollbar presence . on consecutive execution it fails.(i think because some other test case creates a scrollbar) but on individual execution it works.

@dekajp
dekajp commented Nov 16, 2013

i will look why travis build is failed. in my local machine looks like grunt jshint failed with unable to read true file ,but then grunt --force worked

@dekajp dekajp referenced this pull request Nov 16, 2013
Merged

Menu style, experimental #1128

@dekajp dekajp Dialog & Resizable: css styles fixed NE,SE,S,E handles. Fixed #9521 u…
…i.Dialog:Resizable dialogs move/resize out of viewport boundary - which results in scrollbar
f5d5b97
@dekajp
dekajp commented Nov 21, 2013

@jzaefferer - build passed.

@jzaefferer
Member

@mikesherov could you review this one?

@mikesherov
Member

@jzaefferer on it.

@mikesherov
Member

This one makes me nervous because I'm not the greatest with css. @scottgonzalez any input here. It seems like the issue just resolves the negative margins causing scrollies. But I don't feel comfortable that this won't cause any visual regressions.

@scottgonzalez
Member

Can't this be as simple as just adding overflow: hidden back to .ui-dialog?

@tjvantoll
Member

overflow: hidden certainly seems to fix this issue: http://jsfiddle.net/tj_vantoll/tKQZ5/.

It was removed in 2c16435 because @scottgonzalez & I couldn't come up with a reason it was there. Apparently there is one.

@dekajp Would you be interested in testing out overflow: hidden and possibly sending a different pull request?

@dekajp
dekajp commented Jan 10, 2014

@scottgonzalez 👍 . i will test this out and send out a new PR

@scottgonzalez scottgonzalez added a commit that referenced this pull request Jan 15, 2014
@scottgonzalez scottgonzalez Dialog: Apply `overflow: hidden` to contain the resize handles
Fixes #9521
Closes gh-1092
(cherry picked from commit 7741c9f)
b6f8ad6
@dekajp
dekajp commented Jan 15, 2014

@scottgonzalez - do you still need the unit test case ? for this or we can ignore it.

@scottgonzalez
Member

I don't think it's necessary, but if others think it's important, we can revisit it.

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