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

Widget modality gets confused by cloned masks #1175

Closed
wants to merge 3 commits into from
Closed

Widget modality gets confused by cloned masks #1175

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50f1239
Stop widget-modality being confused by multiple masks
Sep 10, 2013
5349d21
cleanups suggested by lint
Sep 10, 2013
c16ee9f
try get out of the wet paper bag
Sep 27, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/widget-modality/js/Widget-Modality.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ var WIDGET = 'widget',
BIND_UI = 'bindUI',
SYNC_UI = 'syncUI',
BOUNDING_BOX = 'boundingBox',
CONTENT_BOX = 'contentBox',
VISIBLE = 'visible',
Z_INDEX = 'zIndex',
CHANGE = 'Change',
Expand Down Expand Up @@ -127,6 +126,7 @@ var WIDGET = 'widget',
WidgetModal.CLASSES = MODAL_CLASSES;


WidgetModal._MASK = null;
/**
* Returns the mask if it exists on the page - otherwise creates a mask. There's only
* one mask on a page at a given time.
Expand All @@ -138,14 +138,15 @@ var WIDGET = 'widget',
*/
WidgetModal._GET_MASK = function() {

var mask = Y.one('.' + MODAL_CLASSES.mask),
var mask = WidgetModal._MASK,
win = Y.one('win');

if (mask) {
if (mask && (mask.getDOMNode() !== null) && mask.inDoc()) {
return mask;
}

mask = Y.Node.create('<div></div>').addClass(MODAL_CLASSES.mask);
WidgetModal._MASK = mask;

if (supportsPosFixed) {
mask.setStyles({
Expand Down Expand Up @@ -270,7 +271,7 @@ var WIDGET = 'widget',
*
* @method _focus
*/
_focus : function (e) {
_focus : function () {

var bb = this.get(BOUNDING_BOX),
oldTI = bb.get('tabIndex');
Expand Down Expand Up @@ -430,7 +431,7 @@ var WIDGET = 'widget',
}

if ( ! supportsPosFixed) {
uiHandles.push(Y.one('win').on('scroll', Y.bind(function(e){
uiHandles.push(Y.one('win').on('scroll', Y.bind(function(){
maskNode.setStyle('top', maskNode.get('docScrollY'));
}, this)));
}
Expand Down Expand Up @@ -547,7 +548,7 @@ var WIDGET = 'widget',
*
* @method _afterFocusOnChange
*/
_afterFocusOnChange : function(e) {
_afterFocusOnChange : function() {
this._detachUIHandlesModal();

if (this.get(VISIBLE)) {
Expand Down
41 changes: 41 additions & 0 deletions src/widget-modality/tests/unit/assets/widget-modality-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,47 @@ suite.add(new Y.Test.Case({

modal1.destroy();
modal2.destroy();
},

'WidgetModality should not get distracted by cloned masks': function () {
var modal,
widget,
before_clone,
after_clone,
orig_mask,
test = Y.one('#test'),
test_orig = Y.Node.create('<div/>');

test.append(test_orig);

this.widget = new TestWidget({
modal : true,
render: test_orig
});

this.widget.hide();
orig_mask = Y.one('.yui3-widget-mask');
Assert.areSame(orig_mask, this.widget.get('maskNode');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jinty,

You seem to have forgotten a parentheses at the end of this line, which results in a parse error.

This line should be corrected to:

Assert.areSame(orig_mask, this.widget.get('maskNode'));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Sep 26, 2013 at 02:51:47PM -0700, Ezequiel Rodriguez wrote:

  •    Assert.areSame(orig_mask, this.widget.get('maskNode');

@jinty,

You seem to have forgotten a parentheses at the end of this line, which results in a parse error.

This line should be corrected to:

Assert.areSame(orig_mask, this.widget.get('maskNode'));

Many thanks!

I've just corrected that. I did try interpret the travis CI failure, but
gave up when it just said "SyntaxError: Parse error" without further
context.

Brian Sutherland


// clone the widget
// typically this would happen because other code clones the
// widget and does not clean up. i.e. a drag-drop proxy
before_clone = test_orig.cloneNode(true);
test.prepend(before_clone);

after_clone = test_orig.cloneNode(true);
test.append(after_clone);

Assert.areNotSame(orig_mask, Y.one('.yui3-widget-mask'));
Assert.areSame(orig_mask, this.widget.get('maskNode');

// if we show the widget, only the original mask is shown
this.widget.show();

Assert.areSame('block', orig_mask.getStyle('display'));
Assert.areSame('none', before_clone.one('.yui3-widget-mask').getStyle('display'));
Assert.areSame('none', after_clone.one('.yui3-widget-mask').getStyle('display'));

}
}));

Expand Down