fix(ui): resolve text overlapping on resize in trace comparison#3505
fix(ui): resolve text overlapping on resize in trace comparison#3505aryunewaskar77-art wants to merge 10 commits intojaegertracing:mainfrom
Conversation
Signed-off-by: aryunewaskar77-art <aryu.newaskar77@gmail.com>
|
Hi @aryunewaskar77-art, thanks for your contribution! To ensure quality reviews, we limit how many concurrent open PRs new contributors can open. This PR is currently on hold (Status: 2/1 open). We will automatically move this into the review queue once your existing PRs are merged or closed. Please see our Contributing Guidelines for details on our tiered quota policy. |
| white-space: nowrap; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| min-width: fit-content; |
There was a problem hiding this comment.
Conflicting CSS properties: min-width: fit-content prevents the element from shrinking below its content width, which contradicts overflow: hidden and text-overflow: ellipsis on lines 31-32. The ellipsis will never show because the element won't shrink to trigger overflow.
Fix: Remove this line:
- min-width: fit-content;Or if minimum width is needed, use a fixed pixel value:
min-width: 80px; /* or appropriate minimum */| min-width: fit-content; | |
| min-width: 80px; /* allow truncation while maintaining minimum width */ |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Signed-off-by: aryunewaskar77-art <aryu.newaskar77@gmail.com>
Signed-off-by: aryunewaskar77-art <aryu.newaskar77@gmail.com>
|
PR quota unlocked! @aryunewaskar77-art, this PR has been moved out of the waiting room and into the active review queue. Thank you for your patience. Current Status: 1/1 open. |
|
While reviewing the diff, I noticed that most of the LOC increase comes from updates to If it was incidental (e.g. local yarn install/version differences), reverting it would significantly reduce noise and make the PR easier to review. |
Signed-off-by: aryunewaskar77-art <aryu.newaskar77@gmail.com>
|
Thanks for flagging this — yarn.lock was added incidentally during local setup and isn’t part of upstream. I’ve removed it from the PR to keep the change focused on the UI fix. |
yurishkuro
left a comment
There was a problem hiding this comment.
the main issue is resolved by #3401. How are the other changes here (to App/*) related to the issue?
|
In order to make it a permanent solution, I added that when resized to small size the text will be shortened till it can be and the rest will be "..." |
Signed-off-by: Aaryaa Newaskar <aryu.newaskar77@gmail.com>
…aceHeader.css Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com> Signed-off-by: Aaryaa Newaskar <aryu.newaskar77@gmail.com>
|
PR quota unlocked! @aryunewaskar77-art, this PR has been moved out of the waiting room and into the active review queue. Thank you for your patience. Current Status: 1/1 open. |
There was a problem hiding this comment.
Pull request overview
This PR resolves a UI issue where text elements overlap when resizing the browser window on the Trace Comparison page. The changes introduce responsive CSS styling and restructured layout components to ensure text properly truncates with ellipsis and layout elements maintain proper spacing across different screen sizes.
Changes:
- Restructured TraceHeader component to separate title text from action buttons, enabling independent flex behavior
- Added responsive CSS rules including text-overflow, white-space, and flex properties to prevent overlapping
- Updated TopNav flex properties and alignment for better responsive behavior
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceHeader.tsx | Restructured JSX to separate trace title text and action elements into distinct divs for independent layout control |
| packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceHeader.css | Added new CSS classes and properties to handle text overflow, ellipsis, and flexible layout behavior |
| packages/jaeger-ui/src/components/TraceDiff/TraceDiffHeader/TraceDiffHeader.css | Added white-space and overflow properties to prevent text wrapping and clipping |
| packages/jaeger-ui/src/components/App/TraceIDSearchInput.css | Added width constraints and text-overflow handling for the trace ID search input |
| packages/jaeger-ui/src/components/App/TopNav.tsx | Updated flex properties and added alignItems for improved responsive navigation layout |
| packages/jaeger-ui/src/components/App/TopNav.css | Added comprehensive responsive CSS rules including media queries for smaller screens |
| packages/jaeger-ui/src/components/App/Branding.css | Added flex-shrink, white-space, and text-overflow properties to prevent branding text from causing layout issues |
| jaeger-uiAryu | New submodule commit reference added |
Comments suppressed due to low confidence (1)
jaeger-uiAryu:1
- This appears to be an accidental commit of a submodule file with an unusual name 'jaeger-uiAryu'. This file should not be part of a UI layout fix PR and should be removed from the commit.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .TraceDiffHeader--traceTitle > span { | ||
| display: inline; | ||
| max-width: 100%; |
There was a problem hiding this comment.
The CSS selector .TraceDiffHeader--traceTitle > span targets direct child span elements, but the JSX was changed to use div elements instead. This rule may now be unused or ineffective since there are no longer direct span children of .TraceDiffHeader--traceTitle.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
jaeger-uiAryu:1
- This new top-level file looks like an accidental submodule/gitlink artifact (a lone
Subproject commit ...line). If this repo is not intentionally adding a new submodule pointer namedjaeger-uiAryu, please remove it from the PR; otherwise, add context in the PR description and ensure the repository’s submodule configuration reflects it.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className="TraceDiffHeader--traceHeader" data-testid="TraceDiffHeader--traceHeader"> | ||
| <h1 className="TraceDiffHeader--traceTitle"> | ||
| <span> | ||
| <div className="TraceDiffHeader--traceTitleText"> |
There was a problem hiding this comment.
Invalid heading markup: an h1 element should not contain div (flow content). This can break HTML semantics and some assistive tech behavior. Prefer using span/strong (phrasing content) wrappers inside the h1, or move the flex layout container outside the h1 (e.g., make the flex wrapper a div and keep the h1 for text only).
| </div> | ||
| <div className="TraceDiffHeader--traceTitleActions"> | ||
| {traceID && <TraceTimelineLink traceID={traceID} />} | ||
| <IoChevronDown className="TraceDiffHeader--traceTitleChevron" /> | ||
| </div> | ||
| </h1> |
There was a problem hiding this comment.
Invalid heading markup: an h1 element should not contain div (flow content). This can break HTML semantics and some assistive tech behavior. Prefer using span/strong (phrasing content) wrappers inside the h1, or move the flex layout container outside the h1 (e.g., make the flex wrapper a div and keep the h1 for text only).
| .TraceDiffHeader--traceTitleText { | ||
| flex: 1; | ||
| overflow: hidden; | ||
| text-overflow: ellipsis; | ||
| white-space: nowrap; | ||
| } |
There was a problem hiding this comment.
For flex children, text truncation with ellipsis commonly fails unless the flex item is allowed to shrink below its content size. Add min-width: 0; to .TraceDiffHeader--traceTitleText so the ellipsis reliably triggers and the title can shrink without pushing/overlapping adjacent actions.
| .TraceDiffHeader--traceTitle > span { | ||
| display: inline; | ||
| max-width: 100%; |
There was a problem hiding this comment.
This selector targets a direct child span of .TraceDiffHeader--traceTitle, but the updated markup no longer renders a direct span child (it now uses .TraceDiffHeader--traceTitleText / .TraceDiffHeader--traceTitleActions). Update the selector to match the new structure (or remove it if no longer needed) to avoid dead/ineffective styling.
yurishkuro
left a comment
There was a problem hiding this comment.
what do branding and TopNav have to do with how trace diffs are styled?
Fixes #3375
This PR fixes a UI issue in the Trace Comparison page where text overlaps when the browser window is resized to smaller widths.
The overlapping text made the comparison view hard to read during responsive resizing. The update ensures that text and layout adjust correctly across different screen sizes.
What changed :
Before :

After :

How this was tested :
3 .Verified text no longer overlaps and layout remains readable