feat(app): version intégrable #43

matt-litwiller · 2023-08-22T15:09:46Z

Please check if the PR fulfills these requirements

The commit message follows our guidelines:https://github.com/infra-geo-ouverte/igo2-quebec/blob/master/.github/CONTRIBUTING.md#git-commit-guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

What is the current behavior? (You can also link to an open issue here)
The standard version of igo2-québec

What is the new behavior?
An embedded version has been added. It can be accessed through configurations and when enabled, the standard version is no longer applied. Instead, the user can select to use the map, filters (new), and a list (new) to present entities in a new manner - similar to what has been done with the Vigilance project. Documentation can be found in the wiki: https://github.com/infra-geo-ouverte/igo2-quebec/wiki/Version-Int%C3%A9grable

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

[ ] Yes
[x] No

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

Other information:

list still is empty - working on figuring out why

filters section appears but list does not. Problem with selectedWorkspace not being recognized (whereas it is recognized in the igo2/igo2-lib version)

workspaces are still not working

config exists for entities instead of retrieving it from the workspace.

entities still not functional, otherwise major features are functional

performance improvemenent for sort-by and page-size mat-selects, and added Maxime's fixes to use the real entities for the entities in the list (instead of the hardcoded values in the configs)

… minor problems) First problem, clicking an element in the list centers on the element correctly, but the map is not refreshed and is "blurry" (error in console) until the user clicks on the map (which causes a refresh). Second, ExpressionChangedAfterItHasBeenCheckedError occurs at line 121 in portal.component.html

fitted map has a lot of space in the bottom which contains no features, something may be off with the fitFeatures() function or with the css related to the map

list and filters are now declared in the app instead of in the portal

…onfigs Possible layouts: filters + map + list, filters + map, map + list, map only (original version)

- clics sur les entités dans la carte améliorés (plusieurs entités peuvent être sélectionnés en même temps et seulement les entités visibles sur la carte peuvent être sélectionnées) - le compte (dans filtres) est rendu indépendant de la liste - plusieurs configurations retravaillées et fonctionnels: filtres + liste, filtres + carte parmi d'autres - "N/D" ajouté pour certains attributs qui n'ont pas eu de retour de terrAPI - à la fois dans les options de filtrage (filtres) et dans les attributs dans la liste

this ensures the uniqueKey must be valid before using it to create filters from terrAPI types

- filtering (with mapserver) fixed bug where the filter string became exponentially longer for terrapi types - "aucun élément" appears when no element is shown in the list - filters can only be used once the terrAPI types have been initialized (to avoid searching for entities that do not exist yet) - messageService used for when selectedEntities are overridden and map clicked before data is initialized - messageService appearance changed to match "guide de design" - some configurations renamed - layout configuration logic changed to support the supported layouts and outlined in the wiki

- removing dead code, comments, etc. - added a few scss lines that should not have been removed from a previous version

- if a terrapi error occurs, no terrAPI type will be used in the application - small ui tweak for the paginator to remain within the list's width

src/app/pages/list/simple-feature-list.component.ts

alecarn

C'est un très gros morceau, bravo pour ton travail.

alecarn · 2023-08-22T17:42:57Z

src/app/app.component.ts

    this.setManifest();
    this.installPrompt();
    this.pwaService.checkForUpdates();
  }

+  public async ngOnInit() {


Les séquences du "lifecycle" ne peuvent pas être asynchrone car elles sont controllés par Angular. Ton code demeure fonctionnel mais il n'attend pas que les promesses soit résolu. On pourrait enlever les async/await.
https://angular.io/guide/lifecycle-hooks

alecarn · 2023-08-22T17:54:38Z

src/app/app.component.ts

+  @HostListener('window:resize', ['$event'])
+	onResize() {
+  		this.isMobile = window.innerWidth >= 768 ? false : true;
+	}


On pourrait utiliser le service de MediaService de @igo2/core
Dans le ngOnInit on pourrait avoir un abonnement sur l'observable media:
this.mediaService.media$.subscribe(media => this.isMobile = this.mediaService.isMobile());

package-lock.json

alecarn · 2023-08-23T11:55:14Z

src/app/app.component.html

+<app-header *ngIf="hasHeader && !useEmbeddedVersion" #header></app-header>

-<app-menu *ngIf="hasMenu"></app-menu>
+<app-menu *ngIf="hasMenu && !useEmbeddedVersion"></app-menu>


On pourrait wrapper le tout dans une condition if/else.

<ng-container *ngIf="useEmbeddedVersion; else otherContentWithRelevantName"> ...Your html for the embeddedVersion </ng-container> <ng-template #otherContentWithRelevantName> ...You html for the other content with header, menu and portal </ng-template>

alecarn · 2023-08-23T12:03:10Z

src/app/app.component.html

+    <div [ngClass]="isMobile ? 'filters-and-map-mobile' : showSimpleFeatureList ? (showMap || showSimpleFilters ? 'filters-and-map-desktop' : 'filters-and-map-mobile') : 'filters-and-map-only-desktop'">
+
+        <app-simple-filters class="app-simple-filters" *ngIf="showSimpleFilters && workspace"
+        [entitiesAll]="entitiesAll"
+        [entitiesList]="entitiesList"
+        [isMobile]="isMobile"
+        [terrAPITypes]="terrAPITypes"
+        [properties]="properties"
+        [additionalProperties]="additionalProperties"
+        [dataInitialized]="dataInitialized"
+        ></app-simple-filters>
+
+        <app-portal class="app-portal" [ngClass]="hasHeader? 'portal-hasHeader': 'portal'" (workspaceSelected)="setSelectedWorkspace($event)"
+        (mapQueryEvent)="setClickedEntities($event)"
+        [features]="features"
+        [additionalProperties]="additionalProperties"
+        [entitiesAll]="entitiesAll"
+        [entitiesList]="entitiesList"
+        [dataInitialized]="dataInitialized"
+        ></app-portal>
+
+    </div>
+    <div class="list">
+        <app-simple-feature-list *ngIf="workspace && showSimpleFeatureList"
+        [entityStore]="workspace.entityStore"
+        [clickedEntities]="clickedEntities"
+        [terrAPITypes]="terrAPITypes"
+        [properties]="properties"
+        [entitiesAll]="entitiesAll"
+        [entitiesList]="entitiesList"
+        [additionalTypes]="additionalTypes"
+        [additionalProperties]="additionalProperties"
+        (listSelection)="onListSelection($event)"
+        ></app-simple-feature-list>
+    </div>
+</div>


Est-ce qu'on pourrait isoler le code qui concerne le EmbeddedVersion dans une composante. Cela permettrait de mieux segmenter les responsabilités entre le AppComponent et le EmbeddedVersion et d'isoler les futures changements ce qui facilite la maintenance

src/app/pages/portal/portal.component.ts

alecarn · 2023-08-23T14:50:37Z

src/app/pages/portal/portal.component.ts

+  /**
+   * @description used to fit the entities inside the map (by modifying the zoom level based on the extent of the entities)
+   */
+  async fitFeatures() {


Ne semble pas utiliser, expliquer pourquoi avec @todo ou supprimer?

Je l'ai laissé pour Maxime au cas où il voudrait l'utiliser, mais la fonctionalité n'était pas correct encore - le zoom était décalé vers le haut. Plus haut dans le code, le seul appel qui est fait à cette fonction est commentée en raison. C'est mentionné dans le doc que j'envoie à Maxime

alecarn · 2023-08-23T14:53:49Z

src/app/pages/portal/portal.component.ts

+   * @param projection the desired projection for the returned coordinates
+   * @returns Promise for the coordinate array ([longitude, latitude]) in the new projection
+   */
+  public async terrAPICoordReformat(coord: string, projection: string): Promise<number[]> {


Est-ce que cette méthode pourrait se retrouver dans le fichier filters-shared-methods.service?
J'aurais l'impression qu'on pourrait créer un service spécifiquement pour TerrAPI avec toutes les méthodes pour interagir avec ce service? Cela nous permettrait de tout regrouper a la même place et s'il y a des changements a faire de ne pas avoir a chercher partout dans les fichiers

alecarn · 2023-08-23T14:58:42Z

package-lock.json

Ne pas commiter ce fichier si aucun changement n'a été apporté aux dépendances du projet.

alecarn · 2023-08-23T16:15:30Z

src/app/app.component.ts

+    // get all the types from terrAPI
+		await this.getTypesFromTerrAPI().then((terrAPITypes: Array<string>) => {
+			this.terrAPITypes = terrAPITypes;
+		});


On pourrait revoir la stratégie et seulement fetcher les types de TerrAPI lorsqu'on est en EmbeddedVersion. Pour éviter que tous les applications doivent initialiser cette requête.

alecarn · 2023-08-23T18:17:59Z

src/app/app.component.ts

+        if(i % 25 === 0){
+          await this.sleep(1000);
+        }
+        await this.checkTerrAPI(type, coords).then(response => {


Lorsqu'on initialise l'application, on génère environ une centaine de requêtes. Est-ce qu'on aurait moyen d'aller chercher toute les domaines de valeurs d'une seule requête par "type". Sinon est-ce qu'on pourrait rendre cette initialisation asynchrone qui serait seulement déclencher lorsque l'usager clique pour ouvrir la sélection

La problématique avec cette stratégie, c'est quoi doit avoir tous les entités au départ. Est-ce qu'on les aura toujours? est-ce que la quantité va augmenter? Ça va pour une dizaine mais si on doit faire la même analyse une centaine ou des milliers la solution n'est plus viable.

Idéalement je crois que les filtres devrait être indépendant des entités. Les clients pourrait configurer leurs points d'entrée d'API pour aller chercher les domaines de valeurs.

matt-litwiller added 22 commits May 19, 2023 15:00

Create filter & list components

51c4a23

reformatted folder structure for lists & filters

ddee8cf

simple list reformatting

00247d8

list still is empty - working on figuring out why

Combining code from igo2-lib and igo2 into igo2-quebec

f6ebeb7

filters section appears but list does not. Problem with selectedWorkspace not being recognized (whereas it is recognized in the igo2/igo2-lib version)

First commit

662d9bd

workspaces are still not working

First commit

f345883

config exists for entities instead of retrieving it from the workspace.

Demo version

cc0b3aa

entities still not functional, otherwise major features are functional

list works with entities

e7d8d9a

performance improvemenent for sort-by and page-size mat-selects, and added Maxime's fixes to use the real entities for the entities in the list (instead of the hardcoded values in the configs)

fit map to filtered features

39631c8

fitted map has a lot of space in the bottom which contains no features, something may be off with the fitFeatures() function or with the css related to the map

reformatted code to make portal contain only the map

7c7b098

list and filters are now declared in the app instead of in the portal

Update portal.component.ts

cdc00ac

Components are more modular and their display can be controled with c…

6387af5

…onfigs Possible layouts: filters + map + list, filters + map, map + list, map only (original version)

simpleFilters demo 2 version

0241b15

Merge branch 'master' into simpleFilters2

84a6d40

merge master into simpleFilters

b09694b

refactor(app): Code cleanup

3279bcc

bug(simple-filters): condition added for uniqueKey

6d3f16e

this ensures the uniqueKey must be valid before using it to create filters from terrAPI types

refactor(*): code cleanup

aa6c89b

- removing dead code, comments, etc. - added a few scss lines that should not have been removed from a previous version

fix(app): error handling for terrAPI types

3005956

- if a terrapi error occurs, no terrAPI type will be used in the application - small ui tweak for the paginator to remain within the list's width

matt-litwiller added the enhancement New feature or request label Aug 22, 2023

matt-litwiller requested review from LAMM26 and alecarn August 22, 2023 15:09

github-advanced-security bot found potential problems Aug 22, 2023

View reviewed changes

src/app/pages/list/simple-feature-list.component.ts Fixed Show fixed Hide fixed

src/app/pages/list/simple-feature-list.component.ts Fixed Show fixed Hide fixed

src/app/pages/list/simple-feature-list.component.ts Fixed Show fixed Hide fixed

refactor(simple-list): changed email regex

54ffa29

github-advanced-security bot found potential problems Aug 22, 2023

View reviewed changes

src/app/pages/list/simple-feature-list.component.ts Fixed Show fixed Hide fixed

matt-litwiller added 2 commits August 22, 2023 11:42

Update app.module.ts

cd9cfc4

refactor(app): remove angular elements code

525ef99

matt-litwiller added 4 commits August 22, 2023 13:05

Update app.module.ts

e3cbf30

docs(*): code cleanup

8423f95

refactor(simpleFeatureList): reformatted regex

a174c85

refactor(simpleFeatureList): modified interface to match documentation

611169d

alecarn requested changes Aug 23, 2023

View reviewed changes

matt-litwiller added 2 commits August 23, 2023 14:28

refactor(app): added types for feature selection in list

006fdd8

Update config.json

e6cf88f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(app): version intégrable #43

feat(app): version intégrable #43

matt-litwiller commented Aug 22, 2023

alecarn left a comment

alecarn Aug 22, 2023

alecarn Aug 22, 2023

alecarn Aug 23, 2023

alecarn Aug 23, 2023

alecarn Aug 23, 2023

matt-litwiller Aug 23, 2023

alecarn Aug 23, 2023

alecarn Aug 23, 2023

alecarn Aug 23, 2023

alecarn Aug 23, 2023

alecarn Aug 23, 2023

feat(app): version intégrable #43

Are you sure you want to change the base?

feat(app): version intégrable #43

Conversation

matt-litwiller commented Aug 22, 2023

alecarn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment