-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Logs UI][Metrics UI] Move actions to the kibana header #84648
Conversation
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
Thanks for the review @katrin-freihofner
I'll take a look 👍
I think this is intentional (it was like that before). Can someone from @elastic/metrics-ui comment on that? |
@afgomez, @katrin-freihofner yes, it is intentional. Anomaly detection only detects anomalies for inventory. Also, on the settings page if we were to show the "Alerts" button, which alerts flyout would appear -- the one for metrics explorer or the one for inventory? |
Ok, it is probably fine to move forward with this to finish the PR. But we should definitely look into this again soon, I will talk to @katefarrar about it. |
@katrin-freihofner this is fixed now. Do you mind taking another look? |
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.
Looks good! Thank you.
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.
Metrics stuff looks good. Thanks!
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Distributable file count
History
To update your PR or re-run it, just comment with: |
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.
Simple and clean, LGTM
export const HeaderActionMenuProvider: React.FC<Required<ContextProps>> = ({ | ||
setHeaderActionMenu, | ||
children, | ||
}) => { | ||
return ( | ||
<HeaderActionMenuContext.Provider value={{ setHeaderActionMenu }}> | ||
{children} | ||
</HeaderActionMenuContext.Provider> | ||
); | ||
}; |
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.
What's the purpose of wrapping the provider in another component? Could <HeaderActionMenuContext.Provider
just be used directly?
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.
It's a way to abstract the value
away into a meaningful name.
I think moving forward it would be interesting to add an InfraContext
that holds all of these little things instead of having 3-4 wrappers, but that's a story for another PR.
…k-field-to-hot-phase * 'master' of github.com:elastic/kibana: (429 commits) simplify popover open state logic (elastic#85379) [Logs UI][Metrics UI] Move actions to the kibana header (elastic#84648) [Search Source] Do not pick scripted fields if * provided (elastic#85133) [Search] Session SO polling (elastic#84225) [Transform] Replace legacy elasticsearch client (elastic#84932) [Uptime]Refactor header and action menu (elastic#83779) Fix agg select external link (elastic#85380) [ILM] Show forcemerge in hot when rollover is searchable snapshot is enabled (elastic#85292) clear using keyboard (elastic#85042) [GS] add tag and dashboard suggestion results (elastic#85144) [ML] API integration tests - skip GetAnomaliesTableData Add ECS field for event.code. (elastic#85109) [Functional][TSVB] Wait for markdown textarea to be cleaned (elastic#85128) skip flaky suite (elastic#62060) skip flaky suite (elastic#85098) Bump highlight.js to v9.18.5 (elastic#84296) Add `server.publicBaseUrl` config (elastic#85075) [Alerting & Actions ] More debug logging (elastic#85149) [Security Solution][Case] Manual attach alert to a case (elastic#82996) Loosen UUID regex to accept uuidv1 or uuidv4 (elastic#85338) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/index.ts # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase/warm_phase.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/i18n_texts.ts # x-pack/plugins/index_lifecycle_management/server/routes/api/policies/register_create_route.ts
#85416) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
Closes #82823
This PR moves the right-side actions from the tab bar to the kibana header, to align with other plugins.
Before:
After:
Implementation details
The implementation is inspired by #82292 and reuses some of the plugins present there.
We had to expose the
appMountParameters.setHeaderActionMenu
to the UI. We did this through a new context.Moving forward, it can be interesting to create an
InfraContext
to aggregate all these dependencies (similiar to what APM does) instead of nesting contexts within each other.