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.

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

Support changing opacity and showing legend in mobile app #2923

Merged
merged 7 commits into from Sep 26, 2017

Conversation

adube
Copy link
Contributor

@adube adube commented Sep 21, 2017

This PR introduces a "mobile" version of the tooltip that contains the opacity slider and "show/hide legend" that are in the desktop apps.

Instead of a tooltip, a plain div is shown underneath the node in the tree. A "cog" button has been added and is used to toggle on/off this menu.

Todo

  • review
  • use the proper icon for show/hide legend (see image in comment below)
  • slider style
  • move methods to gmf

@adube
Copy link
Contributor Author

adube commented Sep 21, 2017

preview

@adube
Copy link
Contributor Author

adube commented Sep 21, 2017

@ybolognini Would you please tell me the name of the font-awesome icon to use instead?

@fredj fredj self-requested a review September 21, 2017 15:19
@fredj fredj added this to the 2.3 milestone Sep 21, 2017
@ybolognini
Copy link
Member

@adube
The open/close legend icon is fa-th-list

Please note that in the mockup, the two separator lines split completely the layer tree (whole column). The idea here was that the menu splits the layer tree and then comes back when the user closes the menu. We could even add an animation where the layer tree is being split (if easy to do)

@adube
Copy link
Contributor Author

adube commented Sep 21, 2017

The open/close legend icon is fa-th-list

Thanks, I'll make the fix.

Please note that in the mockup, the two separator lines split completely the layer tree (whole column). The idea here was that the menu splits the layer tree and then comes back when the user closes the menu. We could even add an animation where the layer tree is being split (if easy to do)

I'm not sure that's currently possible to do that. Please, anyone correct me if I'm wrong: the current UI is a tree, i.e. elements within elements with indentation. The "menu" (i.e. what's shown when the "cog" button is clicked" is part of the tree and therefore follows the identation.

If we want the menu to be 100% width of the tree, then we would need to remove all the indentation of the tree. Please, confirm if that's what you want.

return this.layer &&
(
(
this.depth === 1 && !this.node['mixed']
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using the quoted notation, can you try to change this.node type from Object to gmfThemes.GmfGroup ?

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member

Choose a reason for hiding this comment

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

with an assert

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't. This is a file from "ngeo". We can't put code from "gmf" there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fredj @sbrunner let me know if this is okay.

Copy link
Member

Choose a reason for hiding this comment

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

@sbrunner can we do an exception for the externs ?

Copy link
Member

Choose a reason for hiding this comment

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

instead of changing the @type here, these two function must be moved from ngeo to gmf

@adube
Copy link
Contributor Author

adube commented Sep 25, 2017

Here's what's remaining to do:

  • apply the same style to the slider that's in the desktop version
  • move the methods from ngeo to gmf, while avoiding the use of ['']

I'll work on this soon.

@adube adube force-pushed the c2c-mobile-app-layer-opacity branch from c027f71 to 5499cd9 Compare September 25, 2017 17:11
@adube adube force-pushed the c2c-mobile-app-layer-opacity branch from 6f9592a to b46f408 Compare September 25, 2017 19:52
@sbrunner
Copy link
Member

Probably just a missing asserts : https://travis-ci.org/camptocamp/ngeo/jobs/279666721#L1028-L1045

@fredj
Copy link
Member

fredj commented Sep 26, 2017

@adube thanks for your work on this; please merge

@adube adube merged commit faf3a99 into master Sep 26, 2017
@adube adube deleted the c2c-mobile-app-layer-opacity branch September 26, 2017 14:08
@adube
Copy link
Contributor Author

adube commented Sep 26, 2017

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants