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): manage layers and groups #1697

Open
wants to merge 65 commits into
base: MSP-NEXT-TEMP
Choose a base branch
from

Conversation

alecarn
Copy link
Collaborator

@alecarn alecarn commented Sep 13, 2024

Breaking changes: This is a major refactor of the way we manage layer in IGO

  • New class 'LayerController' to manage layers and offer 3 types of layers, 'systemLayers', 'baseLayers', 'treeLayers'. The systemLayers is the drawing layer, measurement layer and other vital layers for the library. Note that a systemLayer can also be found in the treeLayers. The treeLayers is the hierarchical representation for the LayerViewer. The list of treeLayers can also be accessed under the flattened variant. Finally these three types of layers are aggregated to provide a unified list names 'all' which allows to have the 'baseLayers', 'systemLayers' and the flattened list of the 'treeLayers'

  • The way we access the layers has also changed, we must go through the LayerController

- The LayerViewer does not allow to display the baseLayer, the 'excludeBaselayers' is removed

@alecarn alecarn requested a review from pelord September 13, 2024 16:14
@alecarn alecarn self-assigned this Sep 13, 2024
@alecarn alecarn marked this pull request as draft September 13, 2024 16:15
@alecarn
Copy link
Collaborator Author

alecarn commented Sep 13, 2024

Je l'ai identifié comme DRAFT parce qu'on est en mode test de notre côté. Ça serait bien que tu jette un oeil en code review tout de même voir si la structure/design vous convient.

Breaking changes: This is a major refactor of the way we manage layer in IGO
- New class 'LayerController' to manage layers and offer 3 types of layers, 'systemLayers', 'baseLayers', 'treeLayers'. The systemLayers is the drawing layer, measurement layer and other vital layers for the library. Note that a systemLayer can also be found in the treeLayers. The treeLayers is the hierarchical representation for the LayerViewer. The list of treeLayers can also be accessed under the flattened variant. Finally these three types of layers are aggregated to provide a unified list names 'all' which allows to have the 'baseLayers', 'systemLayers' and the flattened list of the 'treeLayers'

- The way we access the layers has also changed, we must go through the LayerController

- The LayerViewer does not allow to display the baseLayer, the 'excludeBaselayers' is removed

(cherry picked from commit 5ed250d6ca2b27d904de8d3354c11a981c64bf67)

fix: lint

fix(geo): remove duplicate layer from all

(cherry picked from commit 42e4fa9b739c555f3c0f5b3a098ab14307aaae67)

fix(build): circular dependency revert compilationMode to full

fix(geo): circular dependencies
- refactor Layer/LayerGroup

fix(geo): time-filter-list remove BaseLayer

fix(geo): layer - project custom action in the bottom panel

refactor: change label for group

fix(geo): move layer with linkedLayers add system layer in tree

fix: any

chore: update packages

chore: update packages

fix: test

chore: fix path for build
(cherry picked from commit 760475bbbbe06b6fd1eadbf98e04cc210cc4e7bd)
(cherry picked from commit 27e87389de78721f5f0f0086bdd0673424967771)
(cherry picked from commit ec30c113e21d742e2a4b8ca38803973e38188f1c)
(cherry picked from commit d12325f64f0877df8d684f30e1e2555eddeca4e3)
(cherry picked from commit e4e7fb570d031c7fb65a6d34fea328c4299edb46)
@alecarn alecarn marked this pull request as ready for review October 30, 2024 18:02
Copy link
Member

@pelord pelord left a comment

Choose a reason for hiding this comment

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

Globalement, faire le tour pour ne plus conserver aucune mention du map.addLayer ou autre. Gerer tous les cas deprecated. Il me reste certains trucs a valider, dans le geo/...layer donc d'autres commentaires a suivre.

@@ -56,6 +56,7 @@ export interface FormField<T extends FormFieldInputs = FormFieldInputs>
}

export interface FormFieldOptions {
initialValue?: unknown;
Copy link
Member

Choose a reason for hiding this comment

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

Pas possible de gérer via un observable | async comme dans igo-form?
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Notre besoin était différent.
image

Copy link
Member

Choose a reason for hiding this comment

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

mais justement, le data sert a provider les initialvalues

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Faudrait que tu me donnes un exemple parce que moi ça ne fonctionnait pas de mon côté

Copy link
Member

@pelord pelord left a comment

Choose a reason for hiding this comment

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

Globalement, faire le tour pour ne plus conserver aucune mention du map.addLayer ou autre. Gerer tous les cas deprecated. Il me reste certains trucs a valider, dans le geo/...layer donc d'autres commentaires a suivre.

@@ -0,0 +1,23 @@
// import { ComponentFixture, TestBed } from '@angular/core/testing';

// import { TEST_CONFIG } from '../../../../test-config';
Copy link
Member

Choose a reason for hiding this comment

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

_

Copy link
Member

@pelord pelord left a comment

Choose a reason for hiding this comment

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

voir commentaires.


export interface LayerOptions {
export interface LayerOptions extends BaseLayerOptions {
Copy link
Member

Choose a reason for hiding this comment

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

sémantique, baselayerOption OU base layer option?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

C'est le Base class du LayerOptions, donc je dirais le BaseLayerOptions?

Copy link
Member

Choose a reason for hiding this comment

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

ca porte a confusion ;(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

LayerBaseOptions ou LayerOptionsBase?

}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
add(parent?: LayerGroupBase, _soft?: boolean): void {
Copy link
Member

Choose a reason for hiding this comment

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

a quoi sert le _soft, aussi pour le remove

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je vais revisiter cette partie, c'est un flag pour le softRemove lorsqu'on déplace un layer dans un groupe... Mais c'est un anti-pattern d'avoir une méthode avec un flag comme ça.

abstract lowerLayer(layer: Layer): void;
abstract lowerLayers(layers: Layer[]): void;
abstract moveLayer(layer: Layer, from: number, to: number): void;
/** @deprecated find a way to remove this method. For now we discourage to use it until we find the way to remove it */
Copy link
Member

Choose a reason for hiding this comment

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

pourquoi deprecated?

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.

2 participants