Skip to content

TimeLine initial setup #3301

Merged
offtherailz merged 1 commit intogeosolutions-it:c127_geonode_integrationfrom
baloola:on_start_timeline
Oct 30, 2018
Merged

TimeLine initial setup #3301
offtherailz merged 1 commit intogeosolutions-it:c127_geonode_integrationfrom
baloola:on_start_timeline

Conversation

@baloola
Copy link
Copy Markdown
Contributor

@baloola baloola commented Oct 30, 2018

Description

2 parts:

  • the code used to do nothing if rangeSelector returns nothing, this PR uses the data's domain (if rangeSelector returns nothing ) then continues the stream.
  • Probably caused by the react-visjs-timeline, the component needs some change in its props to re-render. As a workaround collapse/expand does the trick (once the user expands timeline it will rerender).

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 #3251

What is the new behavior?
When rendered, the plugin starts in a collapsed state ( loading pre-saved map or adding a timed layer ).
once the user click the (full size ) button the timeline is expanded and works 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:

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 80.942% when pulling 67d963f on baloola:on_start_timeline into c42db8a on geosolutions-it:c127_geonode_integration.

/**
* when there is no timeline state rangeSelector(getState()) returns undefiend, so instead we use the timeData[id] range
*/
const dataRange = timeData.domain.split('--');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Server may return also a list of values (corner cases, 1 element only or an empty list) you must consider also these 2 cases.
I suggest:

  • in the case of list to get the first and the last
  • in case of single value range should be something arbitrary like 1 day before - 1 day after the single value
  • No data. 1 day before 1 day after today

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 as I understand
loadRangeData(id, timeData, getState) is called in the "updateRangeDataOnRangeChange" epic.
in line 174
const layerIds = Object.keys(timeData).filter(id => timeData[id] && timeData[id].domain && isTimeDomainInterval(timeData[id].domain));

This filter removes :
[x] no data option (there is domain)
[x] single value option ( isTimeDomainInterval(domain) checks if domain.indexOf("--") > 0 ). "but not sure if this works for corners "

so it leaves the list option only. If my understanding is correct is there still a reason for checking these cases ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good Point, querying the histogram is not needed in that case. So maybe initializing range is not needed.
We can only check if single values are shown. I'll check if an issue is present for this point.

@offtherailz offtherailz merged commit c5f670f into geosolutions-it:c127_geonode_integration Oct 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants