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

Backgroundlayerselector opacityslider #3553

Merged
merged 12 commits into from Mar 1, 2018

Conversation

llienher
Copy link
Member

@llienher llienher commented Feb 21, 2018

Add a way to get a static background layer linked to an opacity slider, via a parameter to include in the interface. I.E:

  /**
   * @type {string}
   * @export
   */
  this.bgOpacityOptions = 'Test aus Olten';

The string is the layer label (column name from admin interface when defining the layers).

The component is defined as follow in the html:

<gmf-backgroundlayerselector
    gmf-backgroundlayerselector-map="::mainCtrl.map"
    gmf-backgroundlayer-opacity-options="::mainCtrl.bgOpacityOptions"
    class="dropdown-menu">
</gmf-backgroundlayerselector> 

gmf-backgroundlayer-opacity-options is facultative and if ignored, the application will just set as it was without the feature.

First commit is remove correctly the dimension used in the past but not done completly.
The app now use a group containing the active layer and the one used as overlay for the opacity slider. This group is called "background" and is always index 0 with group "data" at index 1.
Tests are updated to this new structures.

@llienher llienher force-pushed the backgroundlayerselector-opacityslider branch 3 times, most recently from 60a5bc2 to 94d69f5 Compare February 22, 2018 08:23
@llienher llienher force-pushed the backgroundlayerselector-opacityslider branch from 94d69f5 to 3ac6b09 Compare February 22, 2018 08:32
@sbrunner
Copy link
Member

What's this strange pull request for?

@llienher
Copy link
Member Author

@sbrunner EPFL and Schwytz want back the opacity slider for a background layer as it was available in CGXP. Ngeo implementation is extending the current background selector.

@@ -166,9 +175,34 @@ gmf.BackgroundlayerselectorController.prototype.$onInit = function() {
gmf.BackgroundlayerselectorController.prototype.handleThemesChange_ = function() {
this.gmfThemes_.getBgLayers().then((layers) => {
this.bgLayers = layers;

if (this.opacityOptions !== undefined && !this.isThemeLoadedOnce_) {
this.isThemeLoadedOnce_ = true;
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment to explain that

Copy link
Member

@ger-benjamin ger-benjamin left a comment

Choose a reason for hiding this comment

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

Thanks !

@sbrunner
Copy link
Member

We shouldn't do that in 2.2...

- New background group
- General PR cleanup
- Adapt getter for the backgroundlayermanager
@llienher llienher force-pushed the backgroundlayerselector-opacityslider branch from c687e1b to 4fe22dd Compare February 22, 2018 14:42
@llienher llienher force-pushed the backgroundlayerselector-opacityslider branch from 6f17671 to 74c1862 Compare February 26, 2018 13:25
@llienher llienher changed the title [wip] Backgroundlayerselector opacityslider Backgroundlayerselector opacityslider Feb 26, 2018

/**
* @type {?ol.Map}
* @type {?string}
Copy link
Member

Choose a reason for hiding this comment

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

Should be {!string|undefined} instead.

layer.setOpacity(0);
layer.setVisible(true);
const bgGroup = this.ngeoLayerHelper_.getGroupFromMap(map, gmf.BACKGROUNDLAYERGROUP_NAME);
bgGroup.getLayers().insertAt(1, layer);
Copy link
Member

Choose a reason for hiding this comment

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

You should remove all the extra layers (> 1) using a for (i=1; i< bgGroup.getLayers().length) and bgGroup.getLayers().removeAt(i).

Then the method will be callable several times.

@@ -166,9 +175,37 @@ gmf.BackgroundlayerselectorController.prototype.$onInit = function() {
gmf.BackgroundlayerselectorController.prototype.handleThemesChange_ = function() {
this.gmfThemes_.getBgLayers().then((layers) => {
this.bgLayers = layers;

if (this.opacityOptions !== undefined && !this.isThemeLoadedOnce_) {
Copy link
Member

Choose a reason for hiding this comment

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

This method is called several times, it should be normal.
I would avoid messing with the ordering inside getBgLayers so I suggest to duplicate the array before changing it (use slice without arguments).
Then remove the isThemeLoadedOnce condition.

@gberaudo
Copy link
Member

@ybolognini, we are going to migrate EPFL to 2.3 starting this week.
If we merge in 2.2 it will need some work and delay to port it to 2.3.

Can we instead merge it in 2.3?

@jwkaltz
Copy link
Member

jwkaltz commented Feb 27, 2018

@gberaudo it is promised to Schwyz for an upcoming 2.2 version, too.

@gberaudo
Copy link
Member

gberaudo commented Mar 1, 2018

@llienher, there is a test failing:

e 64.0.3282 (Linux 0.0.0) ngeo.BackgroundLayerMgr #set unsets the background layer FAILED
	TypeError: Cannot read property 'setZIndex' of null
	    at ngeo.BackgroundLayerMgr.set (src/services/backgroundlayermgr.js:9:2479)
	    at UserContext.it (test/spec/services/backgroundlayermgr.spec.js:61:30)

@llienher llienher merged commit c95d1e4 into 2.2 Mar 1, 2018
@llienher llienher deleted the backgroundlayerselector-opacityslider branch March 1, 2018 13:27
@sbrunner sbrunner added this to the 2.2 milestone Apr 12, 2018
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

6 participants