Skip to content
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

Add text label to copy buttons on tags #1547

Merged

Conversation

Yushmanth-reddy
Copy link
Contributor

Which problem is this PR solving?

Short description of the changes

  • Previously there we only have icons to copy text and JSON. But now we have implemented plain text along with the icons.
    Screenshot from 2023-07-06 13-23-12

Signed-off-by: Yushmanth <pali@pali-reddy.com>
yurishkuro
yurishkuro previously approved these changes Jul 6, 2023
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great!

@codecov
Copy link

codecov bot commented Jul 6, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (85fc71b) 95.96% compared to head (33123d1) 95.96%.

❗ Current head 33123d1 differs from pull request most recent head 0d265bb. Consider uploading reports for the commit 0d265bb to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1547   +/-   ##
=======================================
  Coverage   95.96%   95.96%           
=======================================
  Files         242      242           
  Lines        7557     7557           
  Branches     1984     1984           
=======================================
  Hits         7252     7252           
  Misses        305      305           
Impacted Files Coverage Δ
.../TraceTimelineViewer/SpanDetail/KeyValuesTable.tsx 97.82% <ø> (ø)
...kages/jaeger-ui/src/components/common/CopyIcon.tsx 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

/>
<CopyIcon
className="KeyValueTable--copyIcon"
className="KeyValueTable--copyIcon copyIcon"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copyIcon - why is this needed? It's not even defined in the css file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my bad I thought to use it in css of keyValueTable.css but later realised to use the css of Copyicon.css. Finally it is of no use.Let me make the changes.

Signed-off-by: Yushmanth <pali@pali-reddy.com>
@yurishkuro yurishkuro enabled auto-merge (squash) July 7, 2023 13:54
@yurishkuro yurishkuro disabled auto-merge July 7, 2023 13:54
@yurishkuro yurishkuro changed the title added plain text to copy buttons Add text label to copy buttons on tags Jul 7, 2023
@yurishkuro yurishkuro merged commit 002932d into jaegertracing:main Jul 7, 2023
5 checks passed
anikdhabal pushed a commit to anikdhabal/jaeger-ui that referenced this pull request Jul 14, 2023
Signed-off-by: Anik Dhabal Babu <adhabal2002@gmail.com>
@Yushmanth-reddy Yushmanth-reddy deleted the making-compact-copy-buttons branch July 29, 2023 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Replace Copy icons with text buttons
2 participants