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.

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

[Controls] Debounce time slider selections #201885

Merged
merged 12 commits into from
Dec 19, 2024

Conversation

paulinashakirova
Copy link
Contributor

@paulinashakirova paulinashakirova commented Nov 26, 2024

Summary

This PR fixes the [Controls] Debounce time slider selections issue.

Previously when the user was dragging the time slider, the dashboard would be updating constantly causing lagging on more complex dashboards. This PR adds a local state that will hold the updating values while the user is dragging, and updates the dashboards once the user stops dragging with a delay of 300.

Screen.Recording.2024-11-27.at.11.21.41.mov

@paulinashakirova paulinashakirova self-assigned this Nov 26, 2024
@paulinashakirova paulinashakirova marked this pull request as ready for review November 27, 2024 10:33
@paulinashakirova paulinashakirova requested a review from a team as a code owner November 27, 2024 10:34
@paulinashakirova paulinashakirova added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas papercut Small "burr" in the product that we should fix. labels Nov 27, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@paulinashakirova paulinashakirova marked this pull request as draft November 27, 2024 13:01
@paulinashakirova paulinashakirova force-pushed the debounce-timeslider-range-control branch from aef5b6c to 924d07d Compare November 27, 2024 13:14
@paulinashakirova paulinashakirova marked this pull request as ready for review November 28, 2024 15:13
@Heenawter Heenawter self-requested a review November 28, 2024 15:16
@paulinashakirova paulinashakirova force-pushed the debounce-timeslider-range-control branch from d3d7d7d to 9ebee93 Compare December 9, 2024 10:44
@paulinashakirova paulinashakirova force-pushed the debounce-timeslider-range-control branch from 464af43 to 983cf27 Compare December 16, 2024 10:25
@paulinashakirova
Copy link
Contributor Author

/ci

Comment on lines +60 to +84
const rangeInput = isAnchored ? (
<TimeSliderAnchoredRange
value={props.value}
onChange={props.onChange}
stepSize={props.stepSize}
ticks={props.ticks}
timeRangeMin={props.timeRangeMin}
timeRangeMax={props.timeRangeMax}
compressed={props.compressed}
value={[displayedValue[0] || timeRangeMin, displayedValue[1] || timeRangeMax]}
onChange={(newValue) => {
setDisplayedValue(newValue as Timeslice);
debouncedOnChange(newValue);
}}
stepSize={stepSize}
ticks={ticks}
timeRangeMin={timeRangeMin}
timeRangeMax={timeRangeMax}
compressed={compressed}
/>
) : (
<TimeSliderSlidingWindowRange
value={props.value}
onChange={props.onChange}
stepSize={props.stepSize}
ticks={props.ticks}
timeRangeMin={props.timeRangeMin}
timeRangeMax={props.timeRangeMax}
compressed={props.compressed}
value={[displayedValue[0] || timeRangeMin, displayedValue[1] || timeRangeMax]}
onChange={(newValue) => {
setDisplayedValue(newValue as Timeslice);
debouncedOnChange(newValue);
}}
stepSize={stepSize}
ticks={ticks}
timeRangeMin={timeRangeMin}
timeRangeMax={timeRangeMax}
compressed={compressed}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should definitely refactor this at some point - there is no reason to have these as two separate components 🤦 Not related to this PR, though - this is just super old code that hasn't been cleaned up in awhile.

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #83 / serverless search UI index details page - search solution developer search index details page API key details should show api key
  • [job] [logs] Jest Tests #11 / StepDefinePackagePolicy default API response should display vars coming from package policy

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
controls 494.7KB 494.8KB +157.0B

History

cc @paulinashakirova

Copy link
Contributor

@Heenawter Heenawter left a comment

Choose a reason for hiding this comment

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

Tested locally + code review - thanks so much for tackling this! It works great 🎉

@paulinashakirova paulinashakirova merged commit 87d15ba into main Dec 19, 2024
8 checks passed
@paulinashakirova paulinashakirova deleted the debounce-timeslider-range-control branch December 19, 2024 16:33
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12416841137

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 19, 2024
## Summary

This PR fixes the [[Controls] Debounce time slider
selections](elastic#193227) issue.

Previously when the user was dragging the time slider, the dashboard
would be updating constantly causing lagging on more complex dashboards.
This PR adds a local state that will hold the updating values while the
user is dragging, and updates the dashboards once the user stops
dragging with a delay of 300.

https://github.com/user-attachments/assets/45bca92e-f92a-4c1f-8417-a0a0818c7415

---------

Co-authored-by: Hannah Mudge <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 87d15ba)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
## Summary

This PR fixes the [[Controls] Debounce time slider
selections](elastic#193227) issue.

Previously when the user was dragging the time slider, the dashboard
would be updating constantly causing lagging on more complex dashboards.
This PR adds a local state that will hold the updating values while the
user is dragging, and updates the dashboards once the user stops
dragging with a delay of 300.


https://github.com/user-attachments/assets/45bca92e-f92a-4c1f-8417-a0a0818c7415

---------

Co-authored-by: Hannah Mudge <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
kibanamachine added a commit that referenced this pull request Dec 19, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Controls] Debounce time slider selections
(#201885)](#201885)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Paulina
Shakirova","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-19T16:33:11Z","message":"[Controls]
Debounce time slider selections (#201885)\n\n## Summary\n\nThis PR fixes
the [[Controls] Debounce time
slider\nselections](#193227)
issue.\n\nPreviously when the user was dragging the time slider, the
dashboard\nwould be updating constantly causing lagging on more complex
dashboards.\nThis PR adds a local state that will hold the updating
values while the\nuser is dragging, and updates the dashboards once the
user stops\ndragging with a delay of
300.\n\n\nhttps://github.com/user-attachments/assets/45bca92e-f92a-4c1f-8417-a0a0818c7415\n\n---------\n\nCo-authored-by:
Hannah Mudge <[email protected]>\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"87d15ba2e1b9f7fef30eee2f202da78fe9635e07","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","v9.0.0","backport:prev-minor","papercut"],"title":"[Controls]
Debounce time slider
selections","number":201885,"url":"https://github.com/elastic/kibana/pull/201885","mergeCommit":{"message":"[Controls]
Debounce time slider selections (#201885)\n\n## Summary\n\nThis PR fixes
the [[Controls] Debounce time
slider\nselections](#193227)
issue.\n\nPreviously when the user was dragging the time slider, the
dashboard\nwould be updating constantly causing lagging on more complex
dashboards.\nThis PR adds a local state that will hold the updating
values while the\nuser is dragging, and updates the dashboards once the
user stops\ndragging with a delay of
300.\n\n\nhttps://github.com/user-attachments/assets/45bca92e-f92a-4c1f-8417-a0a0818c7415\n\n---------\n\nCo-authored-by:
Hannah Mudge <[email protected]>\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"87d15ba2e1b9f7fef30eee2f202da78fe9635e07"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201885","number":201885,"mergeCommit":{"message":"[Controls]
Debounce time slider selections (#201885)\n\n## Summary\n\nThis PR fixes
the [[Controls] Debounce time
slider\nselections](#193227)
issue.\n\nPreviously when the user was dragging the time slider, the
dashboard\nwould be updating constantly causing lagging on more complex
dashboards.\nThis PR adds a local state that will hold the updating
values while the\nuser is dragging, and updates the dashboards once the
user stops\ndragging with a delay of
300.\n\n\nhttps://github.com/user-attachments/assets/45bca92e-f92a-4c1f-8417-a0a0818c7415\n\n---------\n\nCo-authored-by:
Hannah Mudge <[email protected]>\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"87d15ba2e1b9f7fef30eee2f202da78fe9635e07"}}]}]
BACKPORT-->

Co-authored-by: Paulina Shakirova <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) papercut Small "burr" in the product that we should fix. release_note:fix Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants