feat(context): set map zoom level after importing context #1384
Conversation
| @@ -222,6 +222,9 @@ export class IgoMap implements MapBase { | |||
| const center = olproj.fromLonLat(options.center, projection); | |||
| view.setCenter(center); | |||
| } | |||
| if (options.zoom) { | |||
| view.setZoom(options.zoom); | |||
| } | |||
There was a problem hiding this comment.
Pourquoi ne pas editer les options avant d'instancier la classe olView?
J'ai l'impression qu'on applique la logique dans cet ordre,
- Instanciation de la vue avec des paramètres par défaut
- Appliquer la vue à la carte (zoom et center par défaut)
- Modifier le positionnement (zoom et center)
Alors qu'on a toute l'information a porter de main pour l'appliquer immédiatement a l'instanciation.
| @@ -82,7 +82,8 @@ export class ContextImportExportComponent implements OnInit, OnDestroy { | |||
| this.res = this.contextService.getContextFromLayers( | |||
| this.map, | |||
| contextOptions.layers, | |||
| contextOptions.name | |||
| contextOptions.name, | |||
| false | |||
There was a problem hiding this comment.
Pourquoi passer le keepCurrentView a false ici?
There was a problem hiding this comment.
Si tu veux exporter un contexte, j'imagine que naturellement, tu veux reloader la vue lors de l'importation.
- fix(View): avoid reset the center and zoom after view initialization
| const viewOptions: ViewOptions = { constrainResolution: true, ...options }; | ||
|
|
||
| if (options.center) { | ||
| const projection = olproj.createProjection(options.projection, 'EPSG:3857'); |
There was a problem hiding this comment.
P-etre faire attention. Passer par le view controller (ou le get projection) pour définir la projection de la map. Ce n'est pas toujours en 3857
There was a problem hiding this comment.
@alecarn Pourquoi creer une projection?
cela suffirait?
viewOptions.center = olproj.fromLonLat(options.center, this.viewController.olView.getProjection().getCode());
There was a problem hiding this comment.
En passant par le viewCtrl, j'ai l'impression qu'on fait une conversion pour rien. Parce qu'a ce point ci, le viewCtrl représente la vue précédente
et dans les options on pourrait retrouver une nouvelle projection et positionnement/center?
| this.ol.setView(view); | ||
| const viewOptions: ViewOptions = { constrainResolution: true, ...options }; | ||
| if (options.center) { | ||
| viewOptions.center = olproj.fromLonLat(options.center, options.projection); |
There was a problem hiding this comment.
En analysant un peu plus je comprends pourquoi j'avais fait la modification précédente.
Auparavent, on effectuait le setView avec la nouvelle projection. Après l'instanciation de la nouvelle vue, on positionnait la vue ce qui faisait en sorte que View.projection était la bonne.
Maintenant, on passe le positionnement en même temps que l'option de projection, donc la projection du View pourrait ne pas correspondre a la nouvelle projection. Je crois qu'on doit se fier à la projection de l'objet d'options sinon laissé Openlayers faire son fallback sur EPSG:3857. Qu'est-ce que tu en penses?
What is the current behavior? (You can also link to an open issue here)
this behavior related to #981 in igo2
What is the new behavior?
Set map zoom level after importing context
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications:
Other information: