fixing onUiState event handler #21715
Conversation
ppisljar
added
review
Feature:Visualizations
💚 Build Succeeded |
|
Could we please add an test, that changing the |
💔 Build Failed |
| @@ -322,6 +332,32 @@ describe('visualize loader', () => { | |||
|
|
|||
| sinon.assert.calledOnce(spy); | |||
| sinon.assert.calledWith(spy, sinon.match({ timeRange: { from: 'now-10d/d', to: 'now' } })); | |||
|
|
|||
| //VisualizeDataLoader.prototype.fetch.restore() | |||
| sinon.assert.calledOnce(spy); | ||
| sinon.assert.calledWith(spy, sinon.match({ forceFetch: false })); | ||
|
|
||
| //VisualizeDataLoader.prototype.fetch.restore() |
💚 Build Succeeded |
There was a problem hiding this comment.
Code LGTM but after pulling the PR and testing on few charts seems that the behaviour still exists, in particular:
- if you open and close the legend with an absolute time range no data is refetched (correct behaviour)
- if you open and close legend using a relative time range, new data for the touched chart is fetched.
If this is the designed way, feel free to ignore this review and go on. If not we have to find a way to disable forceFetching also on relative time ranges.
Could you also please specify what "resolves partly" means. What are the partial part missing on this PR?
|
@markov00, thats why this only partly resolves the original issue. With relative times the timerange will actually change everytime and a new request will be made. |
|
@ppisljar Great, I've updated the PR description with this. |
|
jenkins, test this |
💚 Build Succeeded |
This reverts commit 23edabc.
resolves partly #21480: avoid refetch when using absolute time ranges. With relative times the timerange will actually change everytime and a new request will be made.
the
uiStatechange event will pass the name of the property that changed as the first parameter to the listener function. We don't want that in this scenario, as the first parameter tofetchAndRenderis theforceFetch.