Permalink
Browse files

Dialog: Switch back to shuffling z-index, but only look at .ui-front …

…siblings.

Fixes #9166 - Dialog: moveToTop implementation resets flash/video/iframe/scroll
Fixes #9364 - Dialog: Click of element with tooltip scrolls the dialog to the top
  • Loading branch information...
jzaefferer committed Sep 27, 2013
1 parent 80b17bb commit e263ebda99f3d414bae91a4a47e74a37ff93ba9c
Showing with 94 additions and 5 deletions.
  1. +17 −4 tests/unit/dialog/dialog_methods.js
  2. +62 −0 tests/visual/dialog/stacking.html
  3. +4 −0 tests/visual/index.html
  4. +11 −1 ui/jquery.ui.dialog.js
@@ -144,8 +144,8 @@ test("moveToTop", function() {
expect( 5 );
function order() {
var actual = $( ".ui-dialog" ).map(function() {
return +$( this ).find( ".ui-dialog-content" ).attr( "id" ).replace( /\D+/, "" );
}).get().reverse();
return +$( this ).css( "z-index" );
}).get();
deepEqual( actual, $.makeArray( arguments ) );
}
var dialog1, dialog2,
@@ -161,10 +161,23 @@ test("moveToTop", function() {
equal( focusOn, "dialog2" );
}
});
order( 2, 1 );
order( 100, 101 );
focusOn = "dialog1";
dialog1.dialog( "moveToTop" );
order( 1, 2 );
order( 102, 101 );
});
test( "moveToTop: content scroll stays intact", function() {
expect( 2 );
var otherDialog = $( "#dialog1" ).dialog(),
scrollDialog = $( "#form-dialog" ).dialog({
height: 200
});
scrollDialog.scrollTop( 50 );
equal( scrollDialog.scrollTop(), 50 );

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Oct 2, 2013

Member

This fails in all IEs, where the value is sometimes 38, sometimes 40 or whatever it likes. I've now tried to set an explicit height on the dialog content, to make sure its bigger than the dialog and therefore scrollable enough, but that doesn't make any difference. Any ideas what's causing that?

@jzaefferer

jzaefferer Oct 2, 2013

Member

This fails in all IEs, where the value is sometimes 38, sometimes 40 or whatever it likes. I've now tried to set an explicit height on the dialog content, to make sure its bigger than the dialog and therefore scrollable enough, but that doesn't make any difference. Any ideas what's causing that?

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez

scottgonzalez Oct 2, 2013

Member

That's odd. I'll try to dig into this.

@scottgonzalez

scottgonzalez Oct 2, 2013

Member

That's odd. I'll try to dig into this.

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Oct 16, 2013

Member

Still on your list?

@jzaefferer

jzaefferer Oct 16, 2013

Member

Still on your list?

This comment has been minimized.

Show comment
Hide comment
@FutureKode

FutureKode Nov 8, 2013

Any update on a fix for this?

@FutureKode

FutureKode Nov 8, 2013

Any update on a fix for this?

This comment has been minimized.

Show comment
Hide comment
@dekajp

dekajp Nov 10, 2013

Contributor

in MacOS Chrome-30v i get 47 and it fails. where as it works in FF-24v. but this fiddle http://jsfiddle.net/dekajp/8xEUH/ gives me right results in Chrome and FF

@dekajp

dekajp Nov 10, 2013

Contributor

in MacOS Chrome-30v i get 47 and it fails. where as it works in FF-24v. but this fiddle http://jsfiddle.net/dekajp/8xEUH/ gives me right results in Chrome and FF

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez

scottgonzalez Nov 14, 2013

Member

This is fixed with 144268a.

@scottgonzalez

scottgonzalez Nov 14, 2013

Member

This is fixed with 144268a.

otherDialog.dialog( "moveToTop" );
equal( scrollDialog.scrollTop(), 50 );
});
test("open", function() {
@@ -0,0 +1,62 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Dialog Visual Test</title>
<link rel="stylesheet" href="../../../themes/base/jquery.ui.all.css">
<script src="../../../jquery-1.10.2.js"></script>
<script src="../../../ui/jquery.ui.core.js"></script>
<script src="../../../ui/jquery.ui.widget.js"></script>
<script src="../../../ui/jquery.ui.mouse.js"></script>
<script src="../../../ui/jquery.ui.draggable.js"></script>
<script src="../../../ui/jquery.ui.position.js"></script>
<script src="../../../ui/jquery.ui.resizable.js"></script>
<script src="../../../ui/jquery.ui.button.js"></script>
<script src="../../../ui/jquery.ui.dialog.js"></script>
<style>
body {
font-size: 62.5%;
}
</style>
<script>
$(function() {
var iframeDialog = $( "#dialog-iframe" ).dialog({
position: {
my: "right-90 center"
},
height: 400
}),
scrollingDialog = $( "#dialog-scrolling" ).dialog({
maxHeight: 200,
position: {
my: "left+90 center"
}
}),
otherDialog = $( "#dialog-other" ).dialog({
width: 200,
height: 150
});
});
</script>
</head>
<body>
<p>WHAT: Two dialogs, one embedding an iframe, one having just scrollable content.</p>
<p>EXPECTED: When focusing on one or the other dialog, it shouldn't affect how the content is displayed on the other dialog. It shouldn't reload the iframe or reset the scroll.</p>
<div id="dialog-iframe" title="Dialog that embeds an iframe">
<iframe src="animated.html" height="400"></iframe>
</div>
<div id="dialog-scrolling" title="Dialog with scroll">
<p style="height:600px;background:#eee">a bunch of content</p>
</div>
<div id="dialog-other" title="placeholder">Just another dialog to test stacking</div>
</body>
</html>
View
@@ -33,7 +33,11 @@ <h2>Button</h2>
<h2>Dialog</h2>
<ul>
<li><a href="dialog/animated.html">Animations</a></li>
<li><a href="dialog/complex-dialogs.html">Nested dialogs</a></li>
<li><a href="dialog/form.html">Forms in dialogs</a></li>
<li><a href="dialog/performance.html">Performance</a></li>
<li><a href="dialog/stacking.html">Overlapping dialogs</a></li>
</ul>
<h2>Effects</h2>
View
@@ -212,7 +212,17 @@ $.widget( "ui.dialog", {
},
_moveToTop: function( event, silent ) {
var moved = !!this.uiDialog.nextAll(":visible").insertBefore( this.uiDialog ).length;
var moved = false,
zIndicies = this.uiDialog.siblings( ".ui-front:visible" ).map(function() {
return +$( this ).css( "z-index" );
}).get(),
zIndexMax = Math.max.apply( null, zIndicies );
if ( zIndexMax >= +this.uiDialog.css( "z-index" ) ) {
this.uiDialog.css( "z-index", zIndexMax + 1 );
moved = true;
}
if ( moved && !silent ) {
this._trigger( "focus", event );
}

3 comments on commit e263ebd

@tvdeyen

This comment has been minimized.

Show comment
Hide comment
@tvdeyen

tvdeyen Oct 14, 2013

❤️ \o/ ❤️

tvdeyen replied Oct 14, 2013

❤️ \o/ ❤️

@UrVerySpecial

This comment has been minimized.

Show comment
Hide comment
@UrVerySpecial

UrVerySpecial Oct 18, 2013

Thank you! This is a cause that I confused.

UrVerySpecial replied Oct 18, 2013

Thank you! This is a cause that I confused.

@scottgonzalez

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez
Member

scottgonzalez replied Jul 17, 2014

This caused a regression: http://bugs.jqueryui.com/ticket/10138

Please sign in to comment.