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

Fix print layers when opacity background layer is a layer group #4041

Merged
merged 1 commit into from
Jul 19, 2018

Conversation

llienher
Copy link
Member

@llienher llienher commented Jul 18, 2018

Fixes an print issue with opacity background layer when this is a group of layer instead of a single layer element.

When we generate the print layers, we flatten the gmf layers to remote the group, but we did loose the opacity in the process when this value was only on the group. This PR fixes and apply back the opacity to the layer once flatten.

@llienher llienher self-assigned this Jul 18, 2018
* @return {Array.<ol.layer.Layer>} Layers.
* @private
*/
exports.prototype.getFlatLayers_ = function(layer, array) {
if (layer instanceof olLayerGroup) {
exports.prototype.getFlatLayers_ = function(layer, array, computedOpacity) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you use an optional arguments for "computedOpacity" instead of calling this function with "undefined" at line 297 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

After discussion with @gberaudo, we prefer that it is explicit that the value is called as 'undefined' instead of skipping the last argument.

Copy link
Member

Choose a reason for hiding this comment

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

Okay

});
} else {
if (array.indexOf(layer) < 0) {
layer.set('inheritedOpacity', computedOpacity, true);
Copy link
Member

Choose a reason for hiding this comment

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

Can you check in our code if there is a way to indicate (document) that this opacity and should be used ?

@llienher llienher force-pushed the fix_opacity_background_group_with_print branch from 44f7dc1 to 33bbc8c Compare July 19, 2018 10:02
* Return an opacity value for the specified layer.
* Usage: When we flatten a group (getFlatLayers() method), we get only the child layers.
* If opacity is defined on the group, this value is lost.
* Inherited opacity is a custom 'back-up' value that contains the parent element opacity.
Copy link
Member

Choose a reason for hiding this comment

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

The value does not just come from the parent but is computed based on the layer opacity and the one of all its ancestors.

Copy link
Member

Choose a reason for hiding this comment

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

This documentation should be attached to the getFlatLayers_ method.

Copy link
Member

@gberaudo gberaudo left a comment

Choose a reason for hiding this comment

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

See my comments, otherwise LGTM.

* Return an opacity value for the specified layer.
* Usage: When we flatten a group (getFlatLayers() method), we get only the child layers.
* If opacity is defined on the group, this value is lost.
* Inherited opacity is a custom 'back-up' value that contains the parent element opacity.
Copy link
Member

Choose a reason for hiding this comment

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

This documentation should be attached to the getFlatLayers_ method.

@llienher llienher force-pushed the fix_opacity_background_group_with_print branch from 33bbc8c to 90c73e9 Compare July 19, 2018 11:57
@llienher llienher merged commit 7abbedf into 2.3 Jul 19, 2018
@llienher llienher deleted the fix_opacity_background_group_with_print branch July 19, 2018 12:59
@sbrunner sbrunner added this to the 2.3 milestone Jul 31, 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.

4 participants