-
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
[Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage #49781
[Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage #49781
Conversation
…pabilities-fix # Conflicts: # x-pack/legacy/plugins/infra/public/pages/logs/index.tsx
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
💚 Build Succeeded |
💚 Build Succeeded |
@elasticmachine merge upstream |
💚 Build Succeeded |
…kibana into 48180-settings-capabilities-fix
💚 Build Succeeded |
@elasticmachine merge upstream |
💚 Build Succeeded |
@elasticmachine merge upstream |
💚 Build Succeeded |
@elasticmachine update branch |
💚 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 seems to work well, thank you!
helpText={ | ||
{displaySettings === 'metrics' && ( | ||
<EuiDescribedFormGroup | ||
idAria="matricIndices" |
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 was incorrect before, but maybe we can use this opportunity to fix it:
idAria="matricIndices" | |
idAria="metricIndices" |
Are we still planning to merge this? |
…pabilities-fix # Conflicts: # x-pack/legacy/plugins/infra/public/components/source_configuration/fields_configuration_panel.tsx # x-pack/legacy/plugins/infra/public/components/source_configuration/indices_configuration_panel.tsx # x-pack/legacy/plugins/infra/public/pages/logs/index.tsx # x-pack/legacy/plugins/infra/public/pages/logs/settings.tsx
…kibana into 48180-settings-capabilities-fix
💚 Build SucceededTo update your PR or re-run it, just comment with: |
…Page (elastic#49781) * [Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage * Split settings pages in two; add loading screen to settings page * Restore timestamp field to metrics screen Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* master: (23 commits) [Vis: Default editor] Reactify the timelion editor (elastic#52990) [Discover] fix histogram min interval (elastic#53979) [Telemetry] [Monitoring] Only retry fetching usage once monito… (elastic#54309) [docs][APM] Add runtime index config documentation (elastic#53907) [SIEM] Detection engine timeline (elastic#53783) Filter scripted fields preview field list to source fields (elastic#53826) Management - New platform api (elastic#52579) Reset region and Account when switching inventory (elastic#54287) [SIEM] [Case] Case workflow api schema (elastic#51535) Code coverage setup on CI (elastic#49003) [ML] DF Analytics Results: adds link to docs (elastic#54189) Update schemas boolean, byteSize, and duration to coerce strings (elastic#54177) [Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage (elastic#49781) [Canvas] Fixes bugs with autoplay and refresh (elastic#53149) [ML] DF Analytics Classification: ensure confusion matrix can be fetched (elastic#53629) Fix Vega react eslint errors (elastic#54259) Remove non existing codeowners (elastic#54274) use correct type (elastic#54244) [Dashboard] Removing 100% as dshDashboardViewport height (elastic#54263) add `examples/` to no-restricted-path config (elastic#54252) ...
…Page (#49781) (#54296) * [Metrics UI] Pass relevant shouldAllowEdit capabilities into SettingsPage * Split settings pages in two; add loading screen to settings page * Restore timestamp field to metrics screen Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Summary
Fixes #48180
Goes back to using
infrastructure
andlogs
capabilities to determine whether their respective Settings pages should be editable, as opposed tologs
determining it for both.Waiting on issue discussion to determine if this is enough of a fix, or if the Settings Page needs some slight refactoring.Also splits the Settings Page into two different pages, one displaying only Logs settings and one displaying only Metrics settings.
While we're at it, this also displays the "Loading Data Sources" prompt if the user deeplinks or refreshes directly to the Settings page; previously it would just display a blank page until data sources finished loading.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] 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[ ] Unit or functional tests were updated or added to match the most common scenarios[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers