Improve and cleanup chrome helpMenu links#82300
Conversation
| export type ChromeHelpExtensionLinkBase = Pick< | ||
| EuiButtonEmptyProps, | ||
| 'iconType' | 'target' | 'rel' | 'data-test-subj' | ||
| >; |
There was a problem hiding this comment.
Restrict the links base type (which was EuiButtonEmptyProps previously) to the only properties that make sense for the helpMenu links.
| export type ChromeHelpExtensionMenuLink = ExclusiveUnion< | ||
| ChromeHelpExtensionMenuGitHubLink, | ||
| ExclusiveUnion< | ||
| ChromeHelpExtensionMenuDiscussLink, | ||
| ExclusiveUnion<ChromeHelpExtensionMenuDocumentationLink, ChromeHelpExtensionMenuCustomLink> | ||
| > | ||
| >; | ||
| export type ChromeHelpExtensionMenuLink = | ||
| | ChromeHelpExtensionMenuGitHubLink | ||
| | ChromeHelpExtensionMenuDiscussLink | ||
| | ChromeHelpExtensionMenuDocumentationLink | ||
| | ChromeHelpExtensionMenuCustomLink; |
There was a problem hiding this comment.
Removing the ExclusiveUnion adds a (little) more boilerplate in the switch type logic in renderCustomContent, but it's way more readable, and the tsdoc is greatly improved.
| } | ||
| } | ||
|
|
||
| export const HeaderHelpMenu = injectI18n(HeaderHelpMenuUI); |
There was a problem hiding this comment.
The component was using a mix of i18n and intl. Cleaned that
| case 'custom': { | ||
| const { linkType, content: text, href, ...rest } = link; | ||
| return createCustomLink(index, text, addSpacer, { | ||
| href, | ||
| onClick: this.createOnClickHandler(href, navigateToUrl), | ||
| ...rest, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Only functional change of the PR: we now add a onClick handler to custom links to perform SPA navigation when the link is internal.
|
Pinging @elastic/kibana-platform (Team:Platform) |
lukeelmers
left a comment
There was a problem hiding this comment.
Updates to app arch generated docs LGTM
|
@elastic/kibana-operations The PR was building until fb9376e that reintegrated master into it. I looked quite a bit, but I just don't understand what the issue is or how this PR could cause such failure Error from https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/87975/execution/node/106/log/ is ERROR Error: Command failed with exit code 2: /dev/shm/workspace/kibana/node_modules/typescript/bin/tsc -b tsconfig.refs.json --pretty
14:50:43 packages/kbn-test/target/types/jest/utils/enzyme_helpers.d.ts:29:11 - error TS2503: Cannot find namespace 'ReactIntl'.
14:50:43
14:50:43 29 intl: ReactIntl.InjectedIntl;
14:50:43 ~~~~~~~~~
14:50:43
14:50:43
14:50:43 Found 1 error.Could someone take a quick look? |
|
I have no idea what's going on here, but maybe @restrry has some idea why this is happening when building refs. It seems that the |
I also thought that, but adding the |
|
#83803 will address the issue and unblock this PR. |
|
retest |
TinaHeiligers
left a comment
There was a problem hiding this comment.
Code review and locally tested the work.
Functionally, the PR introduces a small change that works as intended. On a larger scale, the PR makes big improvements and cleans up a few areas too.
LGTM!
💚 Build SucceededMetrics [docs]Distributable file count
Page load bundle
History
To update your PR or re-run it, just comment with: |
* Improve and cleanup chrome helpMenu links * update doc due to merge * remove dev dependencies from test plugin * update generated doc after merge * update generated doc * generated doc * generated doc
Summary
Fix #82158
HeaderHelpMenucomponent and associated typesapplication.navigateToUrlunder the hood for custom internal links to avoid a full page refreshChecklist