[Logs+] Replace infra public usages of link-to routes#158362
Conversation
| import { LocatorDefinition, LocatorPublic } from '@kbn/share-plugin/public'; | ||
| import type { LogsLocatorDependencies, LogsLocatorParams } from './logs_locator'; | ||
|
|
||
| const DISCOVER_LOGS_LOCATOR_ID = 'DISCOVER_LOGS_LOCATOR'; |
There was a problem hiding this comment.
we don't need a new locator id since its either serverless or not, and so we will always use 1 locator id.
| import { LocatorDefinition, LocatorPublic } from '@kbn/share-plugin/public'; | ||
| import type { NodeLogsLocatorDependencies, NodeLogsLocatorParams } from './node_logs_locator'; | ||
|
|
||
| const DISCOVER_NODE_LOGS_LOCATOR_ID = 'DISCOVER_NODE_LOGS_LOCATOR'; |
There was a problem hiding this comment.
we don't need a new locator id since its either serverless or not, and so we will always use 1 locator id.
mohamedhamed-ahmed
added
Team:Infra Monitoring UI - DEPRECATED
|
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
Kerry350
left a comment
Kerry350
left a comment
There was a problem hiding this comment.
The two examples in the description work as expected 👍
Just two small questions (one inline):
Under common/formatters/alert_link.ts there is a /app/logs/link-to/default/logs URL in use, should this be updated? (I appreciate the PR says public only, and that the consumer here would be outside of infra).
Otherwise LGTM 👍
| size={'xs'} | ||
| flush={'both'} | ||
| iconType={'popout'} | ||
| href={locators.nodeLogsLocator.getRedirectUrl({ |
There was a problem hiding this comment.
Does this need to be called on each render? (I wonder if this comes with a slight performance hit due to the Base64 encoding).
There was a problem hiding this comment.
you are right, updated it to useMemo 👍
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Absolutely 👍 this is coming with the next PR as its a bigger refactoring and is affecting both Public and Server |
…replace-infra-link-to-routes-with-the-appropriate-locator
…e-locator' of https://github.com/mohamedhamed-ahmed/kibana into 157985-replace-infra-link-to-routes-with-the-appropriate-locator
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
4 participants
part of #157985
📝 Summary
After implementing infra locators to allow navigation to the logs UI we need to replace all usages of the old link-to routes so that we have strongly typed navigation to the logs UI.
This PR focuses on replacing
link-tousages in the Infra Public plugin.✅ Testing
A)
B)
Screen.Recording.2023-05-24.at.18.04.37.mov