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

[SharedUX][Bugfix] Solution nav with no data page #144280

Merged
merged 4 commits into from
Nov 1, 2022

Conversation

majagrubic
Copy link
Contributor

@majagrubic majagrubic commented Oct 31, 2022

Summary

This PR mitigates: #143114

The problem here was that solution nav was not rendered properly on the Observability page when no data page was displayed. The root cause is setting the css prop on a <EuiPageTemplate.Sidebar />. See comment in the original issue for a detailed explanation. EUI has a fix for this, but it is not in KIbana yet.

This PR replaces emotion/react with emotion/css to generate a className instead of the css prop.

observability_solution_nav

Checklist

Delete any items that are not 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

  • Unit or functional tests were updated or added to match the most common scenarios
    - [ ] Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
    - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
    - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
  • This was checked for cross-browser compatibility

@@ -53,11 +51,11 @@ export const withSolutionNav = <P extends TemplateProps>(WrappedComponent: Compo
isMediumBreakpoint || (canBeCollapsed && isLargerBreakpoint && !isSideNavOpenOnDesktop);
const withSolutionNavStyles = WithSolutionNavStyles(euiTheme);
const sideBarClasses = classNames(
'kbnStickyMenu',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@majagrubic majagrubic marked this pull request as ready for review October 31, 2022 21:32
@majagrubic majagrubic requested a review from a team October 31, 2022 21:32
@majagrubic
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

These changes look great! Thanks for adding the context in the PR #143114 (comment)

@majagrubic majagrubic requested a review from a team November 1, 2022 14:39
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1747 1760 +13
esUiShared 216 229 +13
graph 225 238 +13
home 180 193 +13
infra 950 963 +13
management 84 97 +13
observability 411 424 +13
security 469 482 +13
total +104

Async chunks

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

id before after diff
dashboard 379.3KB 379.2KB -126.0B
discover 416.6KB 416.6KB +9.0B
enterpriseSearch 1.7MB 1.8MB +6.4KB
graph 426.6KB 432.9KB +6.3KB
home 151.6KB 157.9KB +6.3KB
infra 970.7KB 976.9KB +6.3KB
kibanaOverview 46.7KB 46.6KB -65.0B
lens 1.3MB 1.3MB -11.0B
management 30.5KB 36.8KB +6.3KB
maps 2.7MB 2.7MB +6.3KB
observability 485.4KB 491.7KB +6.3KB
security 538.3KB 544.7KB +6.4KB
securitySolution 9.5MB 9.5MB +6.3KB
visualizations 246.6KB 246.6KB -75.0B
total +56.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
esUiShared 152.8KB 159.1KB +6.3KB
graph 7.3KB 7.4KB +56.0B
home 10.8KB 10.9KB +56.0B
infra 85.2KB 85.2KB +56.0B
management 9.8KB 9.8KB +56.0B
observability 68.3KB 68.3KB +56.0B
security 57.7KB 57.8KB +56.0B
total +6.7KB
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 57 63 +6
osquery 103 108 +5
securitySolution 439 443 +4
total +17

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 65 71 +6
osquery 104 110 +6
securitySolution 516 520 +4
total +18

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@majagrubic majagrubic merged commit cb86777 into elastic:main Nov 1, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Nov 1, 2022
@majagrubic majagrubic deleted the fix-143114 branch November 1, 2022 22:15
@KOTungseth KOTungseth added the Feature:Observability Overview Relaunch of the Observability Overview Page label Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Observability Overview Relaunch of the Observability Overview Page release_note:fix v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants