-
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
[Serverless nav] Update footer + project settings cloud links #161971
Conversation
…ana into project-settings-cloud-links
Pinging @elastic/appex-sharedux (Team:SharedUX) |
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.
Changes in core_plugins/rendering.ts
LGTM, left a question regarding trailing slashes.
@@ -52,9 +46,19 @@ const navigationTree: NavigationTreeDefinition = { | |||
}, | |||
{ | |||
link: 'dashboards', | |||
getIsActive: ({ pathNameSerialized, prepend }) => { |
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 added this to maintain the navigation active state when going from the list -> detailed view
}, | ||
{ | ||
link: 'visualize', | ||
getIsActive: ({ pathNameSerialized, prepend }) => { |
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 added this to maintain the navigation active state when going from the list to the different visualizations detailed view
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.
Lgtm 👍
And nice other smaller fixes 👏
packages/shared-ux/chrome/navigation/src/ui/components/group_as_link.tsx
Show resolved
Hide resolved
x-pack/plugins/serverless_observability/public/components/side_navigation/index.tsx
Show resolved
Hide resolved
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.
oblt changes LGTM. Thanks for the additional fixes and improvements 🥳
We'll inject them from cloud. Issue elastic#162127
Thanks for the review @kpatticha ! |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @sebelga |
@TattdCodeMonkey I reverted the change. Can you have another look? Cheers! |
In this PR I've added support for a new
"cloudLinkId"
property on the navigation node definition. If thekibana.yml
config has been set for the cloud link it will render in the left nav. If not it won't render.I've also added support for a new type of navigation node: a top level "group" that renders as a "link" and not as an accordion header. As per the design, this allows us to render the
Developer tools
menu item.I've updated the "search" and "observability" navigation tree to include the cloud links and the developer tools link. For the "search" I've removed the developer tools that was in the body.
How to test
serverless.yml
fileyarn serverless oblt
)Partially fixes #160942 (blocked by #162127)