Add link to timeline view from comparison view and selection #313
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Everett Ross <reverett@uber.com>
ghost
assigned 
Codecov Report
@@ Coverage Diff @@
## master #313 +/- ##
==========================================
+ Coverage 82.55% 82.96% +0.41%
==========================================
Files 141 142 +1
Lines 3141 3147 +6
Branches 649 649
==========================================
+ Hits 2593 2611 +18
+ Misses 438 429 -9
+ Partials 110 107 -3
Continue to review full report at Codecov.
|
|
@everett980 Can you add screenshots to this? |
tiffon
suggested changes
Jan 14, 2019
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/DiffSelection.js
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItemTitle.js
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceTimelineLink.js
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceTimelineLink.js
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceTimelineLink.js
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/CohortTable.js
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/CohortTable.js
Outdated
Show resolved
Hide resolved
everett980
changed the title
Signed-off-by: Everett Ross <reverett@uber.com>
Signed-off-by: Everett Ross <reverett@uber.com>
…omparison-view-and-selection
Signed-off-by: Everett Ross <reverett@uber.com>
tiffon
suggested changes
Jan 18, 2019
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItemTitle.js
Show resolved
Hide resolved
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/DiffSelection.js
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceHeader.js
Outdated
Show resolved
Hide resolved
| */ | ||
|
|
||
| .TraceTimelineLink--icon { | ||
| font-size: 1em; |
There was a problem hiding this comment.
Maybe instead of this class, a large?: boolean prop can be added to NewWindow to accommodate the 1.5em it currently uses?
There was a problem hiding this comment.
so NewWindow would have the prop large, defaulted to true. When true it would have a size of 1.5em, when false it would be 1em?
There was a problem hiding this comment.
Yes, but seems like defaulting it to false and update the other uses of NewWindow would make sense if we have a large prop.
There was a problem hiding this comment.
Otherwise, the size would not be affected, e.g. the style would be .TraceTimelineLink--icon.is-large.NewWindowIcon.is-large
| traceName: 'trace-name-0', | ||
| }, | ||
| error: new Error('error-0'), | ||
| state: { |
There was a problem hiding this comment.
I think state is typically a string. Not sure if this was intentionally ?
tiffon
reviewed
Jan 18, 2019
packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/CohortTable.js
Outdated
Show resolved
Hide resolved
Signed-off-by: Everett Ross <reverett@uber.com>
Signed-off-by: Everett Ross <reverett@uber.com>
Signed-off-by: Everett Ross <reverett@uber.com>
…omparison-view-and-selection
vvvprabhakar
pushed a commit
to vvvprabhakar/jaeger-ui
that referenced
this pull request
Jul 5, 2021
…racing#313) * Add link to TraceDiffHeader & CohortTable, Make DiffSelection clickable Signed-off-by: Everett Ross <reverett@uber.com> * Use getUrl util, use NewWindowIcon Signed-off-by: Everett Ross <reverett@uber.com> * Add test for TraceTimelineLink className Signed-off-by: Everett Ross <reverett@uber.com> * Add DiffSelection, ResultItem, TraceHeader tests Signed-off-by: Everett Ross <reverett@uber.com> * Fix typos Signed-off-by: Everett Ross <reverett@uber.com> * Change NewWindowIcon large behavior, remove link header Signed-off-by: Everett Ross <reverett@uber.com> * Fix typo, remove unused TraceTimelineLink className prop Signed-off-by: Everett Ross <reverett@uber.com> * Update snapshots to match enzyme version Signed-off-by: Everett Ross <reverett@uber.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
Short description of the changes
TraceDiffHeader