-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[SIEM] Add chart interactions - update date picker after brush selection on charts #42440
Conversation
💔 Build Failed |
Pinging @elastic/secops |
💔 Build Failed |
💔 Build Failed |
792d8ef
to
79ff522
Compare
narrowDateRange={(min: number, max: number) => { | ||
setTimeout(() => { | ||
setAbsoluteRangeDatePicker({ id: 'global', from: min, to: max }); | ||
}, 500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish I had a better suggestion for this, but FWIW as a user, the delay doesn't feel perceivable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this a-ok as is as the UI doesn't feel laggy or anything, but I wonder what other ways we can get around this?
Two that come to mind would be:
- Use
narrowDateRange
to set a flag that can be used as the dependency for auseEffect
that runs after the state update. - If there's a way to cancel/prevent said state update from propagating within
narrowDateRange
.
Either way, just thoughts -- doesn't appear to be any need to make any modifications :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I didn't think of these options! Those are very interesting ideas, but I haven't had it work yet 😢, I'll follow up the update from charts and improve this accordingly.
💔 Build Failed |
💔 Build Failed |
jenkins, test this please |
yTickFormatter: numberFormatter, | ||
}, | ||
settings: { | ||
onBrushEnd: getOr(() => {}, 'onBrushEnd', config), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jenkins, test this please |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! 😎 I ran this branch locally and interacted with it for ~ 30 minutes, and it behaved as-expected every single time. It feels good to be able to, for example, see a spike in authentication failures and then brush to see what's happening.
LGTM 📈
|
||
const chartHeight = 74; | ||
export const chartDefaultRotation: Rotation = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be exported? I don't see any usages outside this file currently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, let me remove that, Thanks @spong ! You've got eagle eyes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked out, tested locally, and performed code review. Nice clean implementation + tests @angorayc! LGTM! 👍:tada:🚀
I've left a few comments, but no changes required.
In testing I found these two ancillary issues:
- setAbsoluteRangeDatePicker() does not add date range to 'Recently used date ranges' within SuperDatePicker
- There doesn't appear to be any way to cancel a brush action. I tried pressing escape, moving the mouse cursor drastically out of bounds, etc. Didn't create an issue for this as I figured you may want to chat with the charts team first.
💚 Build Succeeded |
…ion on charts (elastic#42440) * add click and brush events * replace setAbsoluteRangeDatePicker * fix type error * remove a not necessary export
…-or-edit-existing-rollup-job * 'master' of github.com:elastic/kibana: (67 commits) [TSVB] Shim new platform (elastic#39169) [Metric Vis] Shim new platform (elastic#42240) [Tag Cloud] Shim new platform (elastic#42348) Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS. Add disk space percentage to node listing (elastic#42145) [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440) Document HTTP service (elastic#42331) [Reporting] Sanitize 409 error log message (elastic#42495) [docs][skip ci] Maps read only access (elastic#35561) [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407) [DOCS] Updates images and content in Dashboard docs (elastic#42500) Allow sorting on multiple columns in Discover (elastic#41918) [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287) [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534) Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298) Increase max-old-space-size for builds (elastic#42218) [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836) [Logs UI][a11y] Announce name of column on remove column button (elastic#41695) Inspector 👉 New Platform (elastic#42164) Make alerting properly space aware (elastic#42081) ...
…s_autocomplete * 'master' of github.com:elastic/kibana: (189 commits) [TSVB] Shim new platform (elastic#39169) [Metric Vis] Shim new platform (elastic#42240) [Tag Cloud] Shim new platform (elastic#42348) Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS. Add disk space percentage to node listing (elastic#42145) [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440) Document HTTP service (elastic#42331) [Reporting] Sanitize 409 error log message (elastic#42495) [docs][skip ci] Maps read only access (elastic#35561) [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407) [DOCS] Updates images and content in Dashboard docs (elastic#42500) Allow sorting on multiple columns in Discover (elastic#41918) [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287) [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534) Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298) Increase max-old-space-size for builds (elastic#42218) [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836) [Logs UI][a11y] Announce name of column on remove column button (elastic#41695) Inspector 👉 New Platform (elastic#42164) Make alerting properly space aware (elastic#42081) ...
Summary
Update date picker after brushing on a chart:
According to the documentation, we are able to pass a function to
onBrushEnd
, and therefore it will execute the function after brushing on a the chart. The function will recieve min and max selection of timestamp if it is a time chart (reference). So in this implementation, I just passnarrowDateRange
all the way down to the chart and it is doing its job.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
[ ] This was checked for breaking API changes and was labeled appropriately[ ] This includes a feature addition or change that requires a release note and was labeled appropriately