Replace goog.fx in sortable directive #3101

pfirpfel · 2017-11-15T16:31:09Z

~~contribs/gmf/src/controllers/abstractmobile.js~~ (I'm going to do this in a separate branch/pr)

src/directives/sortable.js

pfirpfel · 2017-11-16T08:16:11Z

To test sortable.js:

layerorder example
desktop app, use the demo theme

I tested debug and compile on Chrome and Firefox (desktop and mobile). Needs more testing on IE, Edge and Safari (especially mobile)

sbrunner · 2017-11-16T09:43:49Z

third-party/jquery-ui/jquery.ui.touch-punch.js

@@ -0,0 +1,180 @@
+/*!


From where does this file comes from?

http://touchpunch.furf.com/
Jquery-ui does not support touch events. Without this patch it won't work on mobile devices.

Any reason to not using the npm package?
https://www.npmjs.com/package/jquery-ui-touch-punch

My reasoning was to put it to the same place as jquery-ui, since they belong together. We don't use an npm package for jquery-ui either.
The touchpunch code hasn't changed in the last four years, so I don't expect updates.

The reason to don't use the jquery-ui npm it to have a custom build that's not possible with the npm package...

Changed it.

Fixed :-)

fredj · 2017-11-17T07:55:48Z

Makefile

 		-e 's|\.\./third-party/jquery-ui/jquery-ui.min\.css|lib/jquery-ui.min.css|' \
-		-e 's|\.\./node_modules/bootstrap/dist/js/bootstrap.js|lib/bootstrap.min.js|' \


is the removal of bootstrap.jswanted?

Oops, good catch.

fredj · 2017-11-17T08:35:38Z

In comparaison to https://camptocamp.github.io/ngeo/master/examples/contribs/gmf/apps/desktop:

When dragging an open layertree item, it must be (temporary) closed.
When moving an item, there must be a "shadow" box (dotted box)
(new) If possible it would be nice to limit the dragging in the vertical axis only

fredj · 2017-11-17T08:40:28Z

src/directives/sortable.js

-        if (dragListGroup !== null) {
-          dragListGroup.dispose();
-        }
+        const sortableElement = $(element[0]);


minor: index not need, it can be const sortableElement = $(element);

pfirpfel · 2017-11-17T08:40:30Z

I'm looking into this.

When dragging an open layertree item, it must be (temporary) closed.

I never noticed that. I'm going to check where this happened before.

When moving an item, there must be a "shadow" box (dotted box)

Should be simple, see: http://jqueryui.com/sortable/#placeholder

fredj · 2017-11-17T09:21:59Z

Please also add this change:

diff --git a/contribs/gmf/less/desktoplayertree.less b/contribs/gmf/less/desktoplayertree.less
index 85077d1..da6741e 100644
--- a/contribs/gmf/less/desktoplayertree.less
+++ b/contribs/gmf/less/desktoplayertree.less
@@ -8,7 +8,6 @@ gmf-layertree {
   width: 100%;
   ul {
     margin-bottom: 0;
-    height: 100%;
   }
   > :first-child {
     margin-right: @half-app-margin;

It was a workaround for a limitation in goog.fx

fredj · 2017-11-17T09:23:37Z

And in addition to limit the axis to y, what about adding the containment: 'parent' option?

pfirpfel · 2017-11-21T10:52:01Z

Axis and containment don't really complement each other. They do, in regards to the layertree, very similar things.

Containment has the issue that the layertree parent element is very "tight" and leaves no tolerance. This can make replacing the top layer impossible if you grab the layer handle at the top:

Axis has the issue, that you can drag the layer (but not drop) to useless areas above and below the tree:

But using neither is worse, because you can get the layertree tab to scroll:

I suggest we use the "axis: 'y'"-option. Replacing the top layer comes up often enough.

fredj · 2017-11-21T12:21:07Z

yes, go ahead with axis: 'y' only

gberaudo · 2017-11-24T13:10:50Z

@pfirpfel, what is the status here? Is it ready for merging?

pfirpfel · 2017-11-27T09:19:21Z

I'm stuck with a CSS issue.

My goal is to collapse the layer group when it gets dragged, like it does with goog.fx.

The rules from bootstrap for the regular collapsing(-button) look like this:

.collapse {
  display: none;

  &.in      { display: block; }
  tr&.in    { display: table-row; }
  tbody&.in { display: table-row-group; }
}

This class gets added to the ul-element inside the li of the layer group.
To be more specfic than this rule, I'm currently on this:

.ui-sortable .gmf-layertree-node.gmf-layertree-dragger {
  ul.collapse.in {
    display: none;
  }
}

see: https://github.com/pfirpfel/ngeo/blob/replace-goog-fx-collapse/contribs/gmf/less/desktoplayertree.less#L112

In Chrome and Firefox it wins against the collapse.in-class, but the effect is different.
The display:none of collapse hides the element, the ul.collapse.in of mine leaves the empty space there.

I think the issue is, that the drag and drop API (HTML5) creates a picture that gets dragged around. So it isn't the actual DOM element. Or at least a ill-timed snapshot of it.

As an experiment I removed the .in class (would set display:block) from the element during drag via the drag-event-handlers. This had the same effect as the CSS stuff above.

I think I'll leave it as it is. The collapsing is only really necessary on a long list of layergroups.

gberaudo · 2017-11-27T10:06:38Z

Yes @pfirpfel, we will improve in a second step if it is too disturbing with the new way.

fredj · 2017-11-27T13:40:05Z

src/directives/sortable.js

@@ -1,15 +1,14 @@
 goog.provide('ngeo.SortableOptions');


this line can be removed

…ves empty space

fredj

let's merge as it, thanks @pfirpfel

pfirpfel force-pushed the replace-goog-fx branch from c7db5d2 to 785ac7b Compare November 16, 2017 07:44

sbrunner previously requested changes Nov 16, 2017

View reviewed changes

pfirpfel changed the title ~~[WIP] Replace goog.fx~~ Replace goog.fx in sortable directive Nov 16, 2017

pfirpfel requested a review from fredj November 16, 2017 14:52

fredj reviewed Nov 17, 2017

View reviewed changes

pfirpfel force-pushed the replace-goog-fx branch from 6f6851d to 92c533e Compare November 17, 2017 08:02

fredj reviewed Nov 17, 2017

View reviewed changes

fredj mentioned this pull request Nov 21, 2017

Show imported layers first in layertree, remove 100% height on ul #3127

Merged

sbrunner added this to the 2.3 milestone Nov 21, 2017

pfirpfel force-pushed the replace-goog-fx branch 2 times, most recently from 1154fed to 8c7a85e Compare November 22, 2017 15:07

fredj reviewed Nov 27, 2017

View reviewed changes

src/directives/sortable.js Outdated

@@ -1,15 +1,14 @@

goog.provide('ngeo.SortableOptions');

Copy link

Member

fredj Nov 27, 2017

Choose a reason for hiding this comment

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

this line can be removed

pfirpfel force-pushed the replace-goog-fx branch from 23f7f64 to e3d2642 Compare November 29, 2017 07:38

gberaudo added the es6 modules label Dec 5, 2017

Michael Kuenzli added 5 commits December 5, 2017 16:04

Add Sortable interaction to jquery-ui build

5592bd2

Add jquery.ui.touch-punch

40b5aa2

Replace goog.fx in sortable directive

8312f87

Remove trailing whitespace

5f73176

Explain usage of events more clearly

be37620

Michael Kuenzli added 5 commits December 5, 2017 16:06

Load jquery-ui-touch-punch via npm

3b6e7d7

Revert accidental removal of bootstrap dependency

7e466d0

Add placeholder, set axis option, fix options

4a44cf6

Remove rule, since it doesn't collapse and just hides content and lea…

850e068

…ves empty space

Fix error introduced through rebasing

8c3f1ad

fredj force-pushed the replace-goog-fx branch from ffda977 to 8c3f1ad Compare December 5, 2017 15:10

Prevent key renaming, simplify sortable css class name

778bad8

fredj approved these changes Dec 5, 2017

View reviewed changes

fredj merged commit 2999840 into camptocamp:master Dec 6, 2017

pfirpfel deleted the replace-goog-fx branch April 13, 2018 13:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace goog.fx in sortable directive #3101

Replace goog.fx in sortable directive #3101

pfirpfel commented Nov 15, 2017 •

edited

Loading

pfirpfel commented Nov 16, 2017

sbrunner Nov 16, 2017

pfirpfel Nov 16, 2017

sbrunner Nov 16, 2017

pfirpfel Nov 16, 2017

sbrunner Nov 16, 2017

pfirpfel Nov 16, 2017

fredj Nov 17, 2017

pfirpfel Nov 17, 2017

fredj commented Nov 17, 2017 •

edited

Loading

fredj Nov 17, 2017

pfirpfel commented Nov 17, 2017

fredj commented Nov 17, 2017

fredj commented Nov 17, 2017

pfirpfel commented Nov 21, 2017

fredj commented Nov 21, 2017

gberaudo commented Nov 24, 2017

pfirpfel commented Nov 27, 2017

gberaudo commented Nov 27, 2017

fredj Nov 27, 2017

fredj left a comment

		-e 's\|\.\./third-party/jquery-ui/jquery-ui.min\.css\|lib/jquery-ui.min.css\|' \
		-e 's\|\.\./node_modules/bootstrap/dist/js/bootstrap.js\|lib/bootstrap.min.js\|' \

Replace goog.fx in sortable directive #3101

Replace goog.fx in sortable directive #3101

Conversation

pfirpfel commented Nov 15, 2017 • edited Loading

pfirpfel commented Nov 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredj commented Nov 17, 2017 • edited Loading

Choose a reason for hiding this comment

pfirpfel commented Nov 17, 2017

fredj commented Nov 17, 2017

fredj commented Nov 17, 2017

pfirpfel commented Nov 21, 2017

fredj commented Nov 21, 2017

gberaudo commented Nov 24, 2017

pfirpfel commented Nov 27, 2017

gberaudo commented Nov 27, 2017

Choose a reason for hiding this comment

fredj left a comment

Choose a reason for hiding this comment

pfirpfel commented Nov 15, 2017 •

edited

Loading

fredj commented Nov 17, 2017 •

edited

Loading