-
Notifications
You must be signed in to change notification settings - Fork 521
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
Replace deprecated ant-design props from ReferenceButton and KeyValuesTable #1864
Conversation
Signed-off-by: Ansh Goyal <[email protected]>
Signed-off-by: Ansh Goyal <[email protected]>
Signed-off-by: Ansh Goyal <[email protected]>
// KeyValuesTable is displayed as a menu at span level. | ||
// Example: https://github.com/jaegertracing/jaeger-ui/assets/94157520/b518cad9-cb37-4775-a3d6-b667a1235f89 |
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.
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.
what happens if there's only one link, do we still show the sandwich menu or the outgoing link directly? The latter would be more usable (I think it's unusual to have >1 link on a tag)
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.
In case there's one link only, the link is shown directly, instead of DropDown.
Lines 139 to 147 in 1639eb4
if (links?.length === 1) { | |
valueMarkup = ( | |
<div> | |
<LinkValue href={links[0].url} title={links[0].text}> | |
{jsonTable} | |
</LinkValue> | |
</div> | |
); | |
} else if (links && links.length > 1) { |
Codecov ReportAll modified lines are covered by tests ✅
📢 Thoughts on this report? Let us know!. |
// ReferencesButton is displayed as a menu at the span level. | ||
// Example: https://github.com/jaegertracing/jaeger-ui/assets/94157520/2b29921a-2225-4a01-9018-1a1952f186ef |
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.
Signed-off-by: Ansh Goyal <[email protected]>
Which problem is this PR solving?
Description of the changes
colums.render
prop is no longer required to return children as JSX. Instead, it is by default.How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test