Skip to content

fixing the elevation altering bug and rang/playback conflict#3340

Merged
offtherailz merged 1 commit intogeosolutions-it:c127_geonode_integrationfrom
baloola:timeline_localization
Nov 16, 2018
Merged

fixing the elevation altering bug and rang/playback conflict#3340
offtherailz merged 1 commit intogeosolutions-it:c127_geonode_integrationfrom
baloola:timeline_localization

Conversation

@baloola
Copy link
Copy Markdown
Contributor

@baloola baloola commented Nov 16, 2018

Description

  • the timeline epic on(layer_add), overrides the layer dimensions information in the state with one that it gets from describeDomains request, the original elevation data is used to render the elevation component. This PR updates only the time data in the epic.
  • this PR solves an additional bug related to time selection epic which creates a new form of currentTime " it returns curruntTime = "00:00:00/00:00:00" " that causes some issues because other functions expects "00:00:00"

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix

What is the current behavior? (You can also link to an open issue here)
see #3338

What is the new behavior?
elevation data is rendered normally

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes
  • No

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

Other information:

@baloola baloola requested a review from offtherailz November 16, 2018 14:19
@ghost ghost assigned baloola Nov 16, 2018
@baloola baloola added the bug label Nov 16, 2018
@offtherailz offtherailz merged commit e92c750 into geosolutions-it:c127_geonode_integration Nov 16, 2018
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.004%) to 80.818% when pulling 4fe9fd4 on baloola:timeline_localization into eb11252 on geosolutions-it:c127_geonode_integration.

* while maintaning other dimensions information in state.layer,
* it also creates a list of dimensions (from descibeDomains) in state.dimensions.
* */
const timeDimensionData = dimensions.filter(d => d.name === 'time').reduce(el => el);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is going to fail if filtered dimensions array is empty, I usually use lodash head in these cases:

const timeDimensionData = head(dimensions.filter(d => d.name === 'time'));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this whole observer is triggered if dimensions.length > 0 ( line 54 )

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but if dimensions does not contain a time dimension, the filtered array will be empty, and reduce would crash

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@offtherailz I can fix this with the localization PR, or should I do it in a separate PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just tried in a dev console of Chome

[].reduce(e => e)
VM19016:1 Uncaught TypeError: Reduce of empty array with no initial value
    at Array.reduce (<anonymous>)
    at <anonymous>:1:4
(anonymous) @ VM19016:1
[].reduce(e => e, {})
{}

:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants