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

[Bugfix] Consider the opacity of groups for printing #5485

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mind84
Copy link
Contributor

@mind84 mind84 commented Feb 28, 2025

Now the total opacity is considered when printing, instead of the individual layer opacity.

By total opacity I mean the product of opacity of the individual layer with the opacity of the all parent groups (hierarchically), groupAsLayer layers included.

Ticket : #4984

Backport 3.8, 3.9

Funded by Faunalia

@github-actions github-actions bot added this to the 3.10.0 milestone Feb 28, 2025
Copy link
Member

@Gustry Gustry left a comment

Choose a reason for hiding this comment

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

Thanks @mind84.
Just some comments on the code style, as you could see, there some refactoring about POM, eslint, stylelint and coding rules in the past few weeks.

What is left to do to close #4984 ?

@@ -552,6 +552,19 @@ export class LayerItemState extends EventDispatcher {
}
return this._visibility;
}

/**
* Calculate total opacity by including also all parent gropus opacity values
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Calculate total opacity by including also all parent gropus opacity values
* Calculate total opacity by including also all parent groups opacity values

test.describe('Print opacities', () => {
test.beforeEach(async ({ page }) => {
const url = '/index.php/view/map/?repository=testsrepository&project=group_as_layer_opacity';
await gotoMap(url, page);
Copy link
Member

Choose a reason for hiding this comment

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

This gotoMap is deprecated, as "new" tests must use POM https://playwright.dev/docs/pom. Look in "playwright/pages"
You can add some locators about the "print" box, such as the scale, the buttons...

Copy link
Member

Choose a reason for hiding this comment

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

Please also add the tag @readonly to your new test.

@mind84
Copy link
Contributor Author

mind84 commented Mar 3, 2025

What is left to do to close #4984 ?

I thinks that's it.
I did not tag the PR with the fixes keyword because I'm not really sure this is really what is needed to solve the issue (but probably yes)

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

Successfully merging this pull request may close these issues.

3 participants