You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by andruhon <gi...@git.apache.org> on 2018/09/26 03:35:36 UTC
[GitHub] wicket pull request #294: WICKET-5552 fix modal mousedown and make modal mor...
GitHub user andruhon opened a pull request:
https://github.com/apache/wicket/pull/294
WICKET-5552 fix modal mousedown and make modal more extendable
Demo to reproduce the issue https://github.com/andruhon/wicket-modal-mousedown-issue
Demo using 9.0.0-SNAPSHOT from maven local to demonstrate the fix:
https://github.com/andruhon/wicket-modal-mousedown-issue/tree/fix-WICKET-5552
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/andruhon/wicket WICKET-5552-fix-modal-mousedown-and-make-it-more-extendable
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/294.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #294
----
commit 3adf1b5144b5948c45100129c6279fa183d85420
Author: Andrew Kondratev <an...@...>
Date: 2018-09-26T02:58:11Z
WICKET-5552 fix modal mousedown and make it more extendable
----
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the issue:
https://github.com/apache/wicket/pull/294
Why so complicated - I've fixed the issue by removing the mouse blocker from modal.js and restricting any dragging on the element itself:
```
diff --git a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
index 64852de..8d94b36 100644
--- a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
+++ b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
@@ -2485,6 +2485,10 @@
mouseDownHandler: function (e) {
e = Wicket.Event.fix(e);
+
+ if (e.target !== this) {
+ return;
+ }
var element = this;
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js
index f683119..5a0f122 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js
@@ -1189,7 +1189,7 @@
"<div class=\"w_left\" id='"+idLeft+"'>"+
"<div class=\"w_right_1\">"+
"<div class=\"w_right\" id='"+idRight+"'>"+
- "<div class=\"w_content_1\" onmousedown=\"Wicket.Event.stop(event);\">"+
+ "<div class=\"w_content_1\" >"+
"<div class=\"w_caption\" id=\""+idCaption+"\">"+
"<a class=\"w_close\" style=\"z-index:1\" href=\"#\"></a>"+
"<h3 id=\""+idCaptionText+"\" class=\"w_captionText\"></h3>"+
```
Can you try that instead?
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the issue:
https://github.com/apache/wicket/pull/294
Ha, that's really some crufty code :)
Just noticed that you filed the issue for Wicket 6.x - note that we will only apply security fixes for that version.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the issue:
https://github.com/apache/wicket/pull/294
I'm still not happy about the change :/.
Note that for Wicket 9 I want to move all Drag-handling from wicket-ajax to modal.js - we don't need it really in the former and for modal we'll have it easier to adjust it.
For now we could allow the start handler to veto the drag:
```
diff --git a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
index 18ab6bd..e71b4e8 100644
--- a/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
+++ b/wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js
@@ -2476,14 +2476,16 @@
var element = this;
+ if (element.wicketOnDragBegin(element, e) === false) {
+ return;
+ }
+
Wicket.Event.stop(e);
if (e.preventDefault) {
e.preventDefault();
}
- element.wicketOnDragBegin(element);
-
element.lastMouseX = e.clientX;
element.lastMouseY = e.clientY;
diff --git a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js
index b00ceec..6d63807 100644
--- a/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js
+++ b/wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/markup/html/modal/res/modal.js
@@ -901,7 +901,12 @@
/**
* Called when dragging has started.
*/
- onBegin: function(object) {
+ onBegin: function(element, event) {
+ // ignore anything inside the content
+ if (jQuery(event.target).closest('.w_content_2').size() ) {
+ return false;
+ }
+
if (this.isIframe() && (Wicket.Browser.isGecko() || Wicket.Browser.isIELessThan11() || Wicket.Browser.isSafari())) {
this.revertList = [];
Wicket.Iframe.documentFix(document, this.revertList);
@@ -1183,7 +1188,7 @@
"<div class=\"w_left\" id='"+idLeft+"'>"+
"<div class=\"w_right_1\">"+
"<div class=\"w_right\" id='"+idRight+"'>"+
- "<div class=\"w_content_1\" onmousedown=\"Wicket.Event.stop(event);\">"+
+ "<div class=\"w_content_1\">"+
"<div class=\"w_caption\" id=\""+idCaption+"\">"+
"<a class=\"w_close\" style=\"z-index:1\" href=\"#\"></a>"+
"<h3 id=\""+idCaptionText+"\" class=\"w_captionText\"></h3>"+
```
---
[GitHub] wicket pull request #294: WICKET-5552 fix modal mousedown and make modal mor...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon closed the pull request at:
https://github.com/apache/wicket/pull/294
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the issue:
https://github.com/apache/wicket/pull/294
It is used by ModalWindow and the Ajax debug console only - if we can agree to remove the latter (has been discussed a few years already), we can just move the code to modal.js
But that's a separate discussion.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on the issue:
https://github.com/apache/wicket/pull/294
Thanks for ace845c412d6ea3117c6f366d9bd91b06c6270da. I'll create another one with extensibility improvements soon.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by svenmeier <gi...@git.apache.org>.
Github user svenmeier commented on the issue:
https://github.com/apache/wicket/pull/294
I've fixed WICKET-5552, please open a new pull request if you think ModalWindow is in need of further extensibility.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on the issue:
https://github.com/apache/wicket/pull/294
Updated with typeof object check.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on the issue:
https://github.com/apache/wicket/pull/294
@svenmeier Do you plan to completely remove the drag handling from wicket-ajax or just to allow the modal.js control it by itself?
---
[GitHub] wicket pull request #294: WICKET-5552 fix modal mousedown and make modal mor...
Posted by solomax <gi...@git.apache.org>.
Github user solomax commented on a diff in the pull request:
https://github.com/apache/wicket/pull/294#discussion_r220426844
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js ---
@@ -2301,6 +2302,10 @@
onDrag = jQuery.noop;
}
+ if (typeof(settings) !== "undefined" && settings.stopDragOnCssSelector) {
--- End diff --
According to the code `settings` should be `object`
Maybe it would be better to change this line to be
`typeof(settings) === 'object'` ?
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on the issue:
https://github.com/apache/wicket/pull/294
Shoud I create a new PR without a ticket name or rewording commit is enough?
---
[GitHub] wicket pull request #294: WICKET-5552 fix modal mousedown and make modal mor...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on a diff in the pull request:
https://github.com/apache/wicket/pull/294#discussion_r220427102
--- Diff: wicket-core/src/main/java/org/apache/wicket/ajax/res/js/wicket-ajax-jquery.js ---
@@ -2301,6 +2302,10 @@
onDrag = jQuery.noop;
}
+ if (typeof(settings) !== "undefined" && settings.stopDragOnCssSelector) {
--- End diff --
True. I'll change it to === 'object'. Will check that it still works and push.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on the issue:
https://github.com/apache/wicket/pull/294
@svenmeier sure, it was the first thing I tried. Unfortunately it prevents the w_captionText of the popup from working as a drag area, which I think is undesired effect.
Potentially can be done as something like `e.target !== this && e.target.className.indexOf('w_captionText') == -1`, but it makes the mouseDownHandler know about css classes of modal so I think the approach with some general stopper class is more universal.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on the issue:
https://github.com/apache/wicket/pull/294
@svenmeier it still exists in 7, 8 and in 9 snapshot. Are there jiras for appropriate versions? Didn't find them.
---
[GitHub] wicket issue #294: WICKET-5552 fix modal mousedown and make modal more exten...
Posted by andruhon <gi...@git.apache.org>.
Github user andruhon commented on the issue:
https://github.com/apache/wicket/pull/294
Is there anything else I should rework here?
---