-
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
[APM] Change custom link from EuiListGroupItem to EuiLink #62742
Conversation
886d346
to
fc87abb
Compare
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
@@ -19,7 +31,7 @@ export const CustomLinkSection = ({ | |||
customLinks: CustomLink[]; | |||
transaction: Transaction; | |||
}) => ( | |||
<SectionLinks> | |||
<ul> |
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.
Should this change not be made in the shared SectionLinks
component?
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 agreed, but due to the deadline of the release I thought to do it here, and create a new issue to replace it later on SectionLinks
, WDYT?
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.
Got a question, but other than that looks good.
Sounds good to me, might want to raise that in the observability group in
case it's an issue for them as well.
Op di 7 apr. 2020 17:16 schreef Cauê Marcondes <[email protected]>:
… ***@***.**** commented on this pull request.
------------------------------
In
x-pack/legacy/plugins/apm/public/components/shared/TransactionActionMenu/CustomLink/CustomLinkSection.tsx
<#62742 (comment)>:
> @@ -19,7 +31,7 @@ export const CustomLinkSection = ({
customLinks: CustomLink[];
transaction: Transaction;
}) => (
- <SectionLinks>
+ <ul>
I agreed, but due to the deadline of the release I thought to do it here,
and create a new issue to replace it later on SectionLinks, WDYT?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#62742 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACWDXHGHFSUAN7NPA7VXSDRLM7URANCNFSM4MC5LDCA>
.
|
I can do it, but the reason why I moved to EuiLink was because of external links. They only use it to internal links |
* master: [APM] Change custom link from EuiListGroupItem to EuiLink (elastic#62742) [Remote Clusters] Update callout and move server_name field (elastic#62352) Removes Pitch Presentation Template from Canvas (elastic#62688) FTR: Enable w3c for chromedriver (elastic#62542) [ML] Disable functional tests [ILM] Skip failing API integration test (elastic#62779) [SIEM] Update beat doc (elastic#61902) [Search] Properly add slash preceding path in async search (elastic#62722) [APM] make sure environment query is correct for service maps… (elastic#62764) Add service map icon for rum-js agent type (elastic#62721) [APM] Service map - fixes irrelevant services on data refresh (elastic#62750) [APM] Service map - Fix taxi edge arrow orientation (elastic#62741) [APM] Prevent error rate alert trigger from rendering NaN (elastic#62754) [EPM] Store map visualizations from the package registry and use saved object ID (elastic#62059) [Alerting] for email action, set tls.rejectUnauthorized: false when secure: false (elastic#62380)
closes #62655