Skip to content

Quick wins regarding optimization on various aspects including performance with the field types/domains reading#3347

Merged
jolevesq merged 7 commits intoCanadian-Geospatial-Platform:developfrom
Alex-NRCan:branch-todo-easy
Mar 17, 2026
Merged

Quick wins regarding optimization on various aspects including performance with the field types/domains reading#3347
jolevesq merged 7 commits intoCanadian-Geospatial-Platform:developfrom
Alex-NRCan:branch-todo-easy

Conversation

@Alex-NRCan
Copy link
Member

@Alex-NRCan Alex-NRCan commented Mar 13, 2026

Description

  • Simplified the code when doing updateFilters with regards to formatting the date string
  • Removed the onGetFieldType and onGetFieldDomain overrides from all layer classes (was doing unnecessary confusing extra work, this is part 2 of a previous optimization last week)
  • Moved common delete function from feature-info-event-processor and data-table-event-processor into a helper function
  • Deleted the configs/validator folder (legacy configs)
  • Fixed the demo-important-layers.html template page (was broken and wouldn't add layers when clicking the 'add' button)
  • Removed legacy layerFilter properties on source objects for EsriDynamic and EsriFeature types
  • Few fixes on the opacity adjustments when using the highlight function:
    • Not doing the zIndex thing anymore
    • Not changing to '100%' opacity the layer we want to highlight anymore, leaving it untouched
    • Better management of the opacity through groups (using new getParent() functions of the layers - adding great optimization patterns with this)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Host March 13 @ 12h : https://alex-nrcan.github.io/geoview/
Host March 13 @ 13h : https://alex-nrcan.github.io/geoview/
Host March 13 @ 15h15 : https://alex-nrcan.github.io/geoview/
Host March 13 @ 17h15 : https://alex-nrcan.github.io/geoview/

  • Test the layer highlights
  • Test the data field types (mostly in details and in data-table)
    • Also test for EsriImage (need help testing this for the recent improvements with the raster functions)
  • Test demo-important-layers.html (add a layer (green button))

Checklist:

  • I have build (rush build) and deploy (rush host) my PR
  • I have connected the issues(s) to this PR
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have created new issue(s) related to the outcome of this PR is needed
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

This change is Reviewable

@Alex-NRCan Alex-NRCan self-assigned this Mar 13, 2026
Copy link
Member Author

@Alex-NRCan Alex-NRCan left a comment

Choose a reason for hiding this comment

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

@Alex-NRCan partially reviewed 32 files.
Reviewable status: 31 of 32 files reviewed, all discussions resolved (waiting on Alex-NRCan).

Copy link
Member

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

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

@jolevesq reviewed 32 files and all commit messages, and made 2 comments.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).


packages/geoview-core/src/api/event-processors/event-processor-children/data-table-event-processor.ts line 171 at r2 (raw file):

   * @private
   */
  static #deleteFromArray<T extends TypeResultSetEntry>(

Nice cleanup to remove duplication!


packages/geoview-core/src/geo/layer/gv-layers/raster/gv-esri-dynamic.ts line 296 at r2 (raw file):

   * @protected
   */
  protected override onGetFieldType(fieldName: string): TypeOutfieldsType {

Another great cleanup, we may want to reintroduce domain support in the future but we will do it another way

Copy link
Member Author

@Alex-NRCan Alex-NRCan left a comment

Choose a reason for hiding this comment

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

@Alex-NRCan reviewed 3 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).

…ta-table-event-processor into a helper function
Fixed the demo-important-layers.html template page (was broken and wouldn't add layers when clicking the 'add' button)
Removed legacy layerFilter properties on source objects for EsriDynamic and EsriFeature types
 - Not doing the zIndex thing anymore
 - Not changing to '100%' opacity the layer we want to highlight anymore, leaving it untouched
 - Better management of the opacity through groups)
Copy link
Member Author

@Alex-NRCan Alex-NRCan left a comment

Choose a reason for hiding this comment

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

@Alex-NRCan reviewed 4 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).

Copy link
Member

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

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

@jolevesq made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Alex-NRCan).


packages/geoview-core/src/api/event-processors/event-processor-children/data-table-event-processor.ts line 158 at r4 (raw file):

        UIEventProcessor.hideTabButton(mapId, 'data-table');
      }

TEST

Copy link
Member

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

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

@jolevesq made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Alex-NRCan).


packages/geoview-core/src/api/event-processors/event-processor-children/feature-info-event-processor.ts line 207 at r2 (raw file):

    // Redirect to helper function
    this.helperDeleteFromArray(featureInfoState.layerDataArray, layerPath, (layerArrayResult) => {

INFORM

Copy link
Member Author

@Alex-NRCan Alex-NRCan left a comment

Choose a reason for hiding this comment

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

@Alex-NRCan reviewed 13 files and all commit messages, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).

Copy link
Member Author

@Alex-NRCan Alex-NRCan left a comment

Choose a reason for hiding this comment

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

@Alex-NRCan reviewed 1 file and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).

Copy link
Member

@DamonU2 DamonU2 left a comment

Choose a reason for hiding this comment

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

@DamonU2 reviewed 41 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Alex-NRCan).


packages/geoview-core/src/api/event-processors/abstract-event-processor.ts line 208 at r6 (raw file):

   * @param {T[]} layerArray - The layer array to work with
   * @param {string} layerPath - The layer path to delete
   * @param {(layerArray: T[]) => void} onDeleteCallback - The callback executed when the array is updated

No types in the JSDocs anymore

@Alex-NRCan Alex-NRCan requested a review from DamonU2 March 16, 2026 15:18
Copy link
Member Author

@Alex-NRCan Alex-NRCan left a comment

Choose a reason for hiding this comment

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

@Alex-NRCan reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on DamonU2).


packages/geoview-core/src/api/event-processors/abstract-event-processor.ts line 208 at r6 (raw file):

Previously, DamonU2 (Damon Ulmi) wrote…

No types in the JSDocs anymore

Done.

Can delete a layer that's not existing (only existing through the config)
Copy link
Member Author

@Alex-NRCan Alex-NRCan left a comment

Choose a reason for hiding this comment

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

@Alex-NRCan reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on DamonU2).

Copy link
Member

@DamonU2 DamonU2 left a comment

Choose a reason for hiding this comment

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

@DamonU2 reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).

Copy link
Member

@jolevesq jolevesq left a comment

Choose a reason for hiding this comment

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

@jolevesq reviewed 16 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Alex-NRCan).

@jolevesq jolevesq requested review from MatthewMuehlhauserNRCan and jolevesq and removed request for jolevesq March 17, 2026 11:26
@jolevesq jolevesq self-requested a review March 17, 2026 11:26
@Canadian-Geospatial-Platform Canadian-Geospatial-Platform locked and limited conversation to collaborators Mar 17, 2026
@Canadian-Geospatial-Platform Canadian-Geospatial-Platform unlocked this conversation Mar 17, 2026
@jolevesq jolevesq merged commit a2f17d4 into Canadian-Geospatial-Platform:develop Mar 17, 2026
4 checks passed
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.

3 participants