Add contextual menu on feature click in gmf-drawfeature #1021

adube · 2016-04-12T14:40:37Z

This PR introduces the ngeo.Menu overlay, which consists in a contextual menu configurable with any number of actions. When an action is clicked, it dispatches an event. It can be closed with a close button, or automatically closed when an action is clicked (optional).

This PR also implements a ngeo.Menu in the gmf-drawfeature directive that is shown when a feature is clicked with the following actions:

Move (works, behaviour is the one from the spec)
Rotate (does nothing for now)
Delete (deletes the feature properly)

Todo

Live example

http://adube.github.io/ngeo/gmf-drawfeature-menu/examples/contribs/gmf/drawfeature.html

adube · 2016-04-12T15:59:41Z

@fgravin Ready for review.

fgravin · 2016-04-12T16:24:41Z

contribs/gmf/src/directives/drawfeature.js

+   */
+  this.menu_ = new ngeo.Menu({
+    actions: [{
+      cls: 'glyphicon glyphicon-move',


We use Font Awesome or custom fonts

fgravin · 2016-04-12T16:31:54Z

Looks good, but please could you use bootstrap styling for the overlay look.
See dropdown-menu class that should display something nicer.
Also, use font-awesome fonts instead of glyphcons.

adube · 2016-04-12T17:34:21Z

@fgravin for the dropdown-menu of Bootstrap, I disagree. Have you looked at its style ? It's made to be shown with absolute positioning (to be a drop-down menu). This is not a drop down menu. It's a plain and simple menu. It would take a lot of dissembling of CSS we don't want. In addition, it doesn't support having an X button. Also, it doesn't support having a title like what we're looking for.

Thoughts ?

adube · 2016-04-12T20:30:23Z

@fgravin I decided to include the "translate" interaction in this PR to have an additional menu item working. It's not yet completed, so you can put your review on hold, if you want.

pgiraud · 2016-04-13T08:03:43Z

We should use bootstrap components and styles as much as possible instead of adding our own styles. The main goal behind this is to be able to customize the global styling, all in once.
What about using panels with list-groups?

http://www.codeply.com/go/Ai9qz7aVyG

adube · 2016-04-13T11:59:35Z

+1

adube · 2016-04-13T15:35:08Z

@fgravin Ready for review.

fgravin · 2016-04-13T15:40:42Z

src/ol-ext/interaction/transform.js

@@ -0,0 +1,279 @@
+goog.provide('ngeo.interaction.Translate');


Why transform for the file name ?

fgravin · 2016-04-13T15:42:39Z

When you click on the arrow icon for the translation, it pans the map.

fgravin · 2016-04-13T15:50:36Z

Looks good, keep going this way.
Some minor comments though

adube · 2016-04-13T17:47:34Z

In order to prevent the map from being "panned" while dragging the center feature, I created a PR in OL3 that fixes this: openlayers/openlayers#5228

It features the possibility to define the layers you want to use as filter for the forEachFeatureAtPixel method. Let's wait for OL3 to review.

adube · 2016-04-14T13:04:17Z

The PR in OL3 has been merged. Ready for final review.

@pgiraud Would you please take care of finalizing the review ? The code itself has already been reviewed, but not the UI. Thanks.

pgiraud · 2016-04-15T07:49:08Z

I'll be happy to review this once #1016 is merged and the current pull request is rebased. I want to see it working with visual selection.
If it's not the case already, please make sure that the menu is hidden when a feature is deselected.

adube · 2016-04-15T15:51:44Z

@pgiraud Ready for review. The live example is up-to-date.

…nsform, delete

adube · 2016-04-18T13:14:54Z

I just rebased onto master.

@fgravin, you can pick this one up from @pgiraud and do the review, please.

fgravin · 2016-04-18T13:29:37Z

What is new since my last comment ? For me it was good !
Also, to make review easier, please keep commit history within the PR. We can squash them in the end when it's ready to merge, but even better, github now provide a feature to auto squash all commits from a PR while merging it. So you don't even have to worry about this anymore.

Thanks

adube · 2016-04-18T13:33:09Z

Excellent. Thanks.

What's new:

prevent text and point to show the drop down menu
make sure that the menu is closed on deselect

There's nothing much in terms of "code" to validate again.

fgravin · 2016-04-18T13:41:11Z

OK LG, is it ready to merge ?

adube · 2016-04-18T13:46:47Z

Yes.

adube force-pushed the gmf-drawfeature-menu branch 2 times, most recently from d6cf905 to 9c8d1e5 Compare April 12, 2016 15:59

fgravin reviewed Apr 12, 2016
View reviewed changes

adube force-pushed the gmf-drawfeature-menu branch from 9c8d1e5 to e5c0f7a Compare April 12, 2016 17:44

fgravin changed the title ~~Add contextual menu on feature click in gmf-drawfeature~~ (WIP) - Add contextual menu on feature click in gmf-drawfeature Apr 13, 2016

adube force-pushed the gmf-drawfeature-menu branch 2 times, most recently from 53b23a4 to f747d85 Compare April 13, 2016 15:30

adube changed the title ~~(WIP) - Add contextual menu on feature click in gmf-drawfeature~~ Add contextual menu on feature click in gmf-drawfeature Apr 13, 2016

adube force-pushed the gmf-drawfeature-menu branch from f747d85 to 6fccad3 Compare April 13, 2016 15:36

fgravin reviewed Apr 13, 2016
View reviewed changes

src/ol-ext/interaction/transform.js

@@ -0,0 +1,279 @@

goog.provide('ngeo.interaction.Translate');

Copy link

Member

fgravin Apr 13, 2016

Choose a reason for hiding this comment

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

Why transform for the file name ?

adube force-pushed the gmf-drawfeature-menu branch 4 times, most recently from e68e70e to 73609b4 Compare April 14, 2016 13:02

adube force-pushed the gmf-drawfeature-menu branch 2 times, most recently from 76b9a00 to fd13dc9 Compare April 15, 2016 15:48

Add contextual menu on feature click in gmf-drawfeature, actions: tra…

756883c

…nsform, delete

adube force-pushed the gmf-drawfeature-menu branch from fd13dc9 to 756883c Compare April 18, 2016 12:50

fgravin merged commit 2cb8371 into camptocamp:master Apr 18, 2016

adube deleted the gmf-drawfeature-menu branch April 18, 2016 15:18

sbrunner added this to the Older milestone Aug 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add contextual menu on feature click in gmf-drawfeature #1021

Add contextual menu on feature click in gmf-drawfeature #1021

adube commented Apr 12, 2016 •

edited

adube commented Apr 12, 2016

fgravin Apr 12, 2016

fgravin commented Apr 12, 2016

adube commented Apr 12, 2016

adube commented Apr 12, 2016

pgiraud commented Apr 13, 2016

adube commented Apr 13, 2016

adube commented Apr 13, 2016

fgravin Apr 13, 2016

fgravin commented Apr 13, 2016

fgravin commented Apr 13, 2016

adube commented Apr 13, 2016

adube commented Apr 14, 2016

pgiraud commented Apr 15, 2016 •

edited

adube commented Apr 15, 2016

adube commented Apr 18, 2016

fgravin commented Apr 18, 2016

adube commented Apr 18, 2016

fgravin commented Apr 18, 2016

adube commented Apr 18, 2016

		@@ -0,0 +1,279 @@
		goog.provide('ngeo.interaction.Translate');

Add contextual menu on feature click in gmf-drawfeature #1021

Add contextual menu on feature click in gmf-drawfeature #1021

Conversation

adube commented Apr 12, 2016 • edited

Todo

Live example

adube commented Apr 12, 2016

fgravin Apr 12, 2016

Choose a reason for hiding this comment

fgravin commented Apr 12, 2016

adube commented Apr 12, 2016

adube commented Apr 12, 2016

pgiraud commented Apr 13, 2016

adube commented Apr 13, 2016

adube commented Apr 13, 2016

fgravin Apr 13, 2016

Choose a reason for hiding this comment

fgravin commented Apr 13, 2016

fgravin commented Apr 13, 2016

adube commented Apr 13, 2016

adube commented Apr 14, 2016

pgiraud commented Apr 15, 2016 • edited

adube commented Apr 15, 2016

adube commented Apr 18, 2016

fgravin commented Apr 18, 2016

adube commented Apr 18, 2016

fgravin commented Apr 18, 2016

adube commented Apr 18, 2016

adube commented Apr 12, 2016 •

edited

pgiraud commented Apr 15, 2016 •

edited