[Discover] Context awareness toolkit#253183
Conversation
davismcphee
added
Feature:Discover
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
cc @davismcphee |
| */ | ||
| export const useStableCallback = <T extends (...args: never[]) => unknown>(fn: T | undefined) => { | ||
| const ref = useRef(fn); | ||
| export const useStableCallback = <T extends (...args: Parameters<T>) => ReturnType<T>>(fn: T) => { |
There was a problem hiding this comment.
I originally wrote this hook a long time ago as part of Unified Histogram, and it was moved to this package a few weeks ago to share. I reimplemented the pattern for part of this PR and a bot suggested I use this instead, which makes sense. But the original version isn't totally safe due to timing around useEffect, so I wanted to update it to a better pattern. I can extract this into a separate PR if there are concerns.
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
jughosta
left a comment
jughosta
left a comment
There was a problem hiding this comment.
LGTM, have not noticed any issues when testing 👍
Thanks for creating the centralized way of managing the provided actions!
| private cachedRootContext: ContextWithProfileId<RootContext>; | ||
| private cachedRootProfile: AppliedProfile; |
There was a problem hiding this comment.
Why do we now need the "cached" versions?
There was a problem hiding this comment.
Sorry, forgot to respond to this. It's because of this change. Calling this.getRootProfile() now resolves the root profile each time instead of sharing one instance, so we can inject the current tab's toolkit. Now we cache it locally in the scoped manager and only resolve again when the root context has actually changed.
iblancof
left a comment
iblancof
left a comment
There was a problem hiding this comment.
The obs-exploration code changes LGTM 👍🏻
Also did a local test, and everything looked good.
rStelmach
left a comment
rStelmach
left a comment
There was a problem hiding this comment.
obs-onboarding part LGTM
peteharverson
left a comment
peteharverson
left a comment
There was a problem hiding this comment.
ML changes for patterns profile LGTM
PhilippeOberti
left a comment
PhilippeOberti
left a comment
There was a problem hiding this comment.
Desk tested and code LGTM
💛 Build succeeded, but was flaky
Failed CI Steps
Test Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
cc @davismcphee |
8 participants
Summary
This PR introduces a context awareness toolkit that gives profile extension points a centralized way to trigger actions in the host context (add filters, open tabs, update queries, etc.):
flowchart LR subgraph Hosts["Host creates toolkit"] MA["Main App"] EM["Embeddable"] CP["Context Page"] end Hosts --> TK["Toolkit<br/><code>addFilter, openInNewTab,<br/>updateESQLQuery, setExpandedDoc, ...</code>"] TK --> SPM["ScopedProfilesManager"] SPM -->|"{ context, toolkit }"| Prof["Profile Accessors<br/><i>cell renderers, chart section,<br/>doc viewer, cell actions, ...</i>"] Prof -->|"toolkit.actions.*"| HostsWhy
Profile extension points need to trigger actions in the host app, but there was no central way to do this. Each host (Discover, embeddable, surrounding docs) passed actions down through extension point params, resulting in a lot of duplication and no consistent interface.
How it works
The toolkit gets injected into profile accessors alongside the existing
contextparam, so extensions receive(prev, { context, toolkit })and can call things liketoolkit.actions.addFilter(...)directly.Each host constructs its own toolkit implementation:
This decouples what profiles can do from how each host implements it, and makes adding new host contexts straightforward. It also lays the foundation for extensible state management for profiles, which can also be centralized and exposed via the toolkit.
Cleanup
DiscoverLayoutby moving filter action code out of the componentPrep work related to #242987.
Checklist
release_note:breakinglabel should be applied in these situations.release_note:*label is applied per the guidelinesbackport:*labels.