-
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
[Stateful sidenav] Update feedback urls #198143
[Stateful sidenav] Update feedback urls #198143
Conversation
/ci |
/ci |
…date-feedback-urls
/ci |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
cc @sebelga |
Pinging @elastic/appex-sharedux (Team:SharedUX) |
@@ -195,7 +195,7 @@ export class NavigationPublicPlugin | |||
} | |||
} | |||
|
|||
if (isProjectNav) { | |||
if (isProjectNav && solutionView !== 'classic') { |
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.
is this to make typescript happy? isProjectNav
will always be true when solutionView !== 'classic'
, i guess unless we introduce a different solutionView value in the future.
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.
Yep, just to make TS happy 😊
Starting backport for target branches: 8.15, 8.16, 8.x |
💔 All backports failed
Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
(cherry picked from commit 89fe548) # Conflicts: # x-pack/plugins/serverless_search/public/plugin.ts # x-pack/plugins/spaces/tsconfig.json
💔 Some backports could not be created
Note: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
(cherry picked from commit 89fe548) # Conflicts: # x-pack/plugins/spaces/tsconfig.json
# Backport This will backport the following commits from `main` to `8.16`: - [[Stateful sidenav] Update feedback urls (#198143)](#198143) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Sébastien Loix","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-31T08:55:49Z","message":"[Stateful sidenav] Update feedback urls (#198143)","sha":"89fe54815d9c25b860b34451f3045d43994ad42e","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:SharedUX","backport:prev-major","Feature:Chrome"],"number":198143,"url":"https://github.com/elastic/kibana/pull/198143","mergeCommit":{"message":"[Stateful sidenav] Update feedback urls (#198143)","sha":"89fe54815d9c25b860b34451f3045d43994ad42e"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198143","number":198143,"mergeCommit":{"message":"[Stateful sidenav] Update feedback urls (#198143)","sha":"89fe54815d9c25b860b34451f3045d43994ad42e"}}]}] BACKPORT-->
# Backport This will backport the following commits from `main` to `8.x`: - [[Stateful sidenav] Update feedback urls (#198143)](#198143) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Sébastien Loix","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-31T08:55:49Z","message":"[Stateful sidenav] Update feedback urls (#198143)","sha":"89fe54815d9c25b860b34451f3045d43994ad42e","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:SharedUX","backport:prev-major","Feature:Chrome"],"number":198143,"url":"https://github.com/elastic/kibana/pull/198143","mergeCommit":{"message":"[Stateful sidenav] Update feedback urls (#198143)","sha":"89fe54815d9c25b860b34451f3045d43994ad42e"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198143","number":198143,"mergeCommit":{"message":"[Stateful sidenav] Update feedback urls (#198143)","sha":"89fe54815d9c25b860b34451f3045d43994ad42e"}},{"url":"https://github.com/elastic/kibana/pull/198479","number":198479,"branch":"8.16","state":"OPEN"}]}] BACKPORT-->
In this PR I've updated the "Let us know" button URL to point to unique URLs based on the side nav solution.
New URLs
Note on implementation
I didn't want to introduce a new state on the ChromeService or create a dependency of the
space
plugin on the SharedUX navigation package.I decided to change the
id
type of the navigation tree from astring
to a strictSolutionId
type.Most of the changes in this PR are to accommodate to this stricter requirements, with very little changes (and thus regression risks) to the implementation (those are mainly in packages/shared-ux/chrome/navigation/src/ui/navigation.tsx and packages/shared-ux/chrome/navigation/src/ui/components/feedback_btn.tsx).