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

feat(geo): Print with resolution PDF and image #1264

Merged
merged 30 commits into from
Jul 4, 2023
Merged

Conversation

aziz-access
Copy link
Contributor

What is the current behavior? (You can also link to an open issue here)

this behavior related to #402 in igo2

What is the new behavior?

Ability to choose the resolution before exporting pdf or image

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications:

Other information:

@aziz-access aziz-access added enhancement New features and improvements // Nouvelles fonctionnalités et améliorations in progress In progress // Développement en cours labels May 24, 2023
@aziz-access aziz-access self-assigned this May 24, 2023
@alecarn alecarn self-requested a review May 29, 2023 13:25
@alecarn
Copy link
Collaborator

alecarn commented May 29, 2023

On ne peut pas faire la revue de cette PR car on a inclus #1256. En stand-by tant que cette PR n'est pas mergé ou corriger cette PR pour ne pas l'inclure.

Copy link
Collaborator

@alecarn alecarn left a comment

Choose a reason for hiding this comment

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

Ça me semble bien beau, plus des commentaires de style pour améliorer la lisibilité et maintenance.

@@ -830,15 +830,28 @@ export class PrintService {
legendPosition: PrintLegendPosition
) {
const status$ = new Subject();
// const resolution = map.ol.getView().getResolution();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dans les paramètres de la méthode, on pourrait ajouter le type pour la résolution: PrintResolution

// Reset original map size
map.ol.setSize(initialMapSize);
map.ol.getView().setResolution(viewResolution);
map.ol.renderSync();
Copy link
Collaborator

Choose a reason for hiding this comment

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

On peut éviter le commentaire en refactorant cette partie et créer un méthode resetOriginalMapSize(size, resolution)

Comment on lines 1035 to 1038
// Set map image size
map.ol.setSize(newMapSize);
const scaling = Math.min(newMapSize[0] / initialMapSize[0], newMapSize[1] / initialMapSize[1]);
map.ol.getView().setResolution(viewResolution / scaling);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pour éviter de laisser des commentaire on pourrait en faire un méthode, cela améliorait aussi la lisibilité.

Suggested change
// Set map image size
map.ol.setSize(newMapSize);
const scaling = Math.min(newMapSize[0] / initialMapSize[0], newMapSize[1] / initialMapSize[1]);
map.ol.getView().setResolution(viewResolution / scaling);
setMapResolution(initialSize: [number, number], resolution: PrintResolution): void {
const scaleFactor = resolution / 96;
const [width, height] = initialSize;
const scaledSize = [Math.round(width * scaleFactor), Math.round(height * scaleFactor)];
map.ol.setSize(scaledSize);
const scaling = Math.min(scaledSize[0] / width, scaledSize[1] / height);
const view = map.ol.getView();
view.setResolution(view.getResolution() / scaling);
}

@alecarn alecarn merged commit f492050 into next Jul 4, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features and improvements // Nouvelles fonctionnalités et améliorations in progress In progress // Développement en cours not ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants