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.

Sign up for GitHub

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

Jump to bottom

WV-3242 index out of bounds error #5332

Merged
merged 4 commits into from
Jul 15, 2024
Merged

WV-3242 index out of bounds error #5332

merged 4 commits into from
Jul 15, 2024

Conversation

PatchesMaps
Copy link
Contributor

Description

When adding more than one TEMPO layer, sometimes there is an "Index out of bounds" error displayed in the console

How To Test

  1. git checkout WV-3242-index-out-of-bounds
  2. npm ci && npm run build && npm run watch
  3. Go to this link
  4. Add several TEMPO L2 layers and verify that there are no index out of bounds errors.

PR Submission Checklist

This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • I have added necessary documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • Any dependent changes have been merged and published in downstream modules (if applicable)

Merging

Please use the squash and merge commit method unless each commit in your branch is vital to the commit history of main.

@nasa-gibs/worldview

@PatchesMaps PatchesMaps self-assigned this Jul 5, 2024
christof-wittreich
christof-wittreich reviewed Jul 5, 2024
View reviewed changes
web/js/mapUI/components/layers/addLayer.js
@@ -51,7 +51,12 @@ function AddLayer(props) {
compareMapUi.create(ui.selected, mode);
} else {
const newLayer = await createLayer(def);
ui.selected.getLayers().insertAt(index, newLayer);
const layers = ui.selected.getLayers();
if (index <= layers.getLength()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like a bit of a hacky fix to me. Why is this behavior different than when adding any other layers? Surely there is a solution to this further upstream that solves the actual problem, rather than accounting for the problem later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure fixing the source of the bug is feasible within the scope of this story. The source of the issue is a race condition caused by this useEffect hook which is unfortunately critical to how AERONET layers are rendered. Modifying said hook to only reload layers when the newly rendered palette is from AERONET works for the most part but unfortunately causes AERONET layers to not render correctly when hydrated from the url.

@minniewong minniewong added this to the v4.44.0 milestone Jul 15, 2024
@PatchesMaps PatchesMaps merged commit 9d84b98 into develop Jul 15, 2024
4 checks passed
christof-wittreich added a commit that referenced this pull request Jul 22, 2024
@christof-wittreich @PatchesMaps @minniewong
Release to Main v4.44.0 (#5361)
469a805 
* Main to Develop v4.43.0 (#5338)

* UAT-v4.42.0

* Release to Main v4.42.0 (#5324)

* WV-3177: Rename EOSDIS Worldview to NASA Worldview (#5316)

* Remove EOSDIS

* Update EOSDIS to ESDIS and LANCE acronyms

* Updated eslint-plugin-n (#5317)

* dependency updates

* WV-3210 Distraction Free Timezone Fix (#5320)

* Added kioskMode check for timezone label

* Separated ternary statements

* Update IMERG to version 7 (#5321)

* UAT-v4.42.0

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>

* v4.43.0

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>

* Dependency Updates 07-12-234 (#5345)

* Added check for non BEST layers in build (#5333)

* WV-3104 GIBS API Link in Embed Tab (#5340)

* Added api docs link

* Simplified to a single class

* WV-3154 Replace Social Tab with Citation Tab (#5346)

* Changed the social tab to the citation tab

* Changed citation to cite us & modified intro text

* Fixed test

* WV-3242 index out of bounds error (#5332)

* only insert layers at valid index

* only reload when new palette is AERONET

* reload layers on change to rendered palettes

* Dependency Updates 07-19-2024 (#5358)

* Dependency Updates 07-19-2024

* revert sass bump

* WV-3220 Smaller Extent BBox For Finding High-Resolution Imagery (#5347)

* Added smallerExtent

* Added getSmallerExtent reusable function

* WV-3218: Update welcome.md to point users to the Worldview Image of the Week (#5359)

* Update welcome.md

* open link in new tab

* v4.44.0

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>
christof-wittreich added a commit that referenced this pull request Jul 22, 2024
@christof-wittreich @PatchesMaps @minniewong
Main to Develop v4.44.0 (#5362)
86b9265 
* UAT-v4.42.0

* Release to Main v4.42.0 (#5324)

* WV-3177: Rename EOSDIS Worldview to NASA Worldview (#5316)

* Remove EOSDIS

* Update EOSDIS to ESDIS and LANCE acronyms

* Updated eslint-plugin-n (#5317)

* dependency updates

* WV-3210 Distraction Free Timezone Fix (#5320)

* Added kioskMode check for timezone label

* Separated ternary statements

* Update IMERG to version 7 (#5321)

* UAT-v4.42.0

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>

* v4.43.0

* Release to Main v4.44.0 (#5361)

* Main to Develop v4.43.0 (#5338)

* UAT-v4.42.0

* Release to Main v4.42.0 (#5324)

* WV-3177: Rename EOSDIS Worldview to NASA Worldview (#5316)

* Remove EOSDIS

* Update EOSDIS to ESDIS and LANCE acronyms

* Updated eslint-plugin-n (#5317)

* dependency updates

* WV-3210 Distraction Free Timezone Fix (#5320)

* Added kioskMode check for timezone label

* Separated ternary statements

* Update IMERG to version 7 (#5321)

* UAT-v4.42.0

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>

* v4.43.0

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>

* Dependency Updates 07-12-234 (#5345)

* Added check for non BEST layers in build (#5333)

* WV-3104 GIBS API Link in Embed Tab (#5340)

* Added api docs link

* Simplified to a single class

* WV-3154 Replace Social Tab with Citation Tab (#5346)

* Changed the social tab to the citation tab

* Changed citation to cite us & modified intro text

* Fixed test

* WV-3242 index out of bounds error (#5332)

* only insert layers at valid index

* only reload when new palette is AERONET

* reload layers on change to rendered palettes

* Dependency Updates 07-19-2024 (#5358)

* Dependency Updates 07-19-2024

* revert sass bump

* WV-3220 Smaller Extent BBox For Finding High-Resolution Imagery (#5347)

* Added smallerExtent

* Added getSmallerExtent reusable function

* WV-3218: Update welcome.md to point users to the Worldview Image of the Week (#5359)

* Update welcome.md

* open link in new tab

* v4.44.0

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>

* Merge branch 'develop'

---------

Co-authored-by: Patrick Moulden <[email protected]>
Co-authored-by: minniewong <[email protected]>
Co-authored-by: PatchesMaps <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christof-wittreich christof-wittreich christof-wittreich approved these changes

@minniewong minniewong minniewong approved these changes

@Tomcariello Tomcariello Tomcariello approved these changes

Assignees

@PatchesMaps PatchesMaps

Labels
None yet
Projects
None yet
Milestone
v4.44.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@PatchesMaps @christof-wittreich @minniewong @Tomcariello