-
Notifications
You must be signed in to change notification settings - Fork 591
Enhance TraceDiff UI components #2806
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
Enhance TraceDiff UI components #2806
Conversation
Signed-off-by: Parship Chowdhury <[email protected]>
aa8a8a8 to
579b06f
Compare
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Parship Chowdhury <[email protected]>
packages/jaeger-ui/src/components/QualityMetrics/Header.test.js
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2806 +/- ##
=======================================
Coverage 97.31% 97.32%
=======================================
Files 256 256
Lines 7947 7951 +4
Branches 2077 2078 +1
=======================================
+ Hits 7734 7738 +4
Misses 213 213 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
packages/jaeger-ui/src/components/TraceDiff/TraceDiffGraph/TraceDiffGraph.css
Outdated
Show resolved
Hide resolved
Signed-off-by: Parship Chowdhury <[email protected]>
|
please resolve merge conflict |
Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
Signed-off-by: Parship Chowdhury <[email protected]>
|
the new chevron position looks a bit confusing to me. I think top-right corner was the right position. Next to the trace title/ID it is interpreted as "select", but next to timestamp etc. it reads more like "show more details". |
|
@yurishkuro The changes are proper text wrapping in small windows, consistent chevron icon positioning, cean layout in both window sizes Please review these changs. I'll push them once you confirm. |
e6d2c6c
|
@yurishkuro I had actually sent you a message earlier asking for confirmation, I was working on for the chevron icon and text wrapping in different window sizes, since you told me the new chevron position looks a bit confusing. I've attached screenshots in my last msg. The last change of chevron icon was not pushed. Would you like me to create a new PR for the change or the current chevron icon position is ok? |
|
I thought you added the change - please do in another PR |
## Which problem is this PR solving? - Related to #2806 ## Description of the changes  ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [ ] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: Parship Chowdhury <[email protected]>




Which problem is this PR solving?
Description of the changes
UI Enhancements:
Additional Changes(Non UI):
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run testBefore:
After:
Before:
After: