Add expand/collapse toggle for input/output messages in span overview#1070
Open
chamals3n4 wants to merge 5 commits into
Open
Add expand/collapse toggle for input/output messages in span overview#1070chamals3n4 wants to merge 5 commits into
chamals3n4 wants to merge 5 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesCollapsible MessageList in span details Overview
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
console/workspaces/pages/traces/src/subComponents/spanDetails/Overview.tsx (2)
138-156: ⚡ Quick winUse a semantic control for the section toggle header.
The toggle is bound to a clickable
Box, which is not announced as an interactive control. Prefer rendering the header as a semantic button (or move toggle handling onto a dedicated button witharia-expanded/aria-controls) so assistive tech gets correct state and intent.Suggested patch
- <Box + <Box + component="button" + type="button" sx={{ display: "flex", alignItems: "center", justifyContent: "space-between", mb: isExpanded ? 2 : 0, - cursor: "pointer", - userSelect: "none", + width: "100%", + border: 0, + p: 0, + m: 0, + backgroundColor: "transparent", + textAlign: "left", + cursor: "pointer", + userSelect: "none", }} + aria-expanded={isExpanded} + aria-controls={`${testId ?? title}-content`} onClick={() => setIsExpanded((prev) => !prev)} > <Typography variant="h6">{title}</Typography> <IconButton size="small" aria-label={isExpanded ? "Collapse" : "Expand"} > {isExpanded ? <ChevronUp /> : <ChevronDown />} </IconButton> </Box> - <Collapse in={isExpanded}> + <Collapse in={isExpanded} id={`${testId ?? title}-content`}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/workspaces/pages/traces/src/subComponents/spanDetails/Overview.tsx` around lines 138 - 156, The section header toggle is currently implemented with a non-semantic Box element that has onClick and click-related styling, which assistive technologies cannot properly announce as an interactive control. Replace the Box click handler approach by making the IconButton component the primary toggle control: move the onClick handler from the Box to the IconButton, add aria-expanded attribute set to the isExpanded state value, and optionally add aria-controls pointing to the collapsible content if it has an id. This ensures screen readers announce it as a toggle button with the correct expanded/collapsed state. Remove the pointer cursor and click-related styling from the Box since the IconButton will be the semantic control.
157-229: ⚡ Quick winConsider unmounting collapsed message content to reduce hidden render cost.
Given this section can contain large payloads, keeping collapsed children mounted can still carry rendering/DOM overhead.
unmountOnExitis a low-cost improvement here.Suggested patch
- <Collapse in={isExpanded}> + <Collapse in={isExpanded} unmountOnExit>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/workspaces/pages/traces/src/subComponents/spanDetails/Overview.tsx` around lines 157 - 229, The Collapse component wrapping the Stack with the messages.map() call keeps its children mounted even when collapsed, which causes unnecessary rendering overhead for potentially large payloads. Add the unmountOnExit prop to the Collapse component to ensure the collapsed content is removed from the DOM rather than just hidden, reducing rendering cost.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@console/workspaces/pages/traces/src/subComponents/spanDetails/Overview.tsx`:
- Around line 138-156: The section header toggle is currently implemented with a
non-semantic Box element that has onClick and click-related styling, which
assistive technologies cannot properly announce as an interactive control.
Replace the Box click handler approach by making the IconButton component the
primary toggle control: move the onClick handler from the Box to the IconButton,
add aria-expanded attribute set to the isExpanded state value, and optionally
add aria-controls pointing to the collapsible content if it has an id. This
ensures screen readers announce it as a toggle button with the correct
expanded/collapsed state. Remove the pointer cursor and click-related styling
from the Box since the IconButton will be the semantic control.
- Around line 157-229: The Collapse component wrapping the Stack with the
messages.map() call keeps its children mounted even when collapsed, which causes
unnecessary rendering overhead for potentially large payloads. Add the
unmountOnExit prop to the Collapse component to ensure the collapsed content is
removed from the DOM rather than just hidden, reducing rendering cost.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d1a78d1-ee24-4d1c-81b1-0f226a4040d1
📒 Files selected for processing (1)
console/workspaces/pages/traces/src/subComponents/spanDetails/Overview.tsx
Author
|
UPDATE : make input messages expand by default, feel better UX than having both collapsed |
Author
|
hi @RAVEENSR , can you check this PR please? |
Contributor
|
Hi @chamals3n4 , can you use the Oxygen UI Accordion component for this implimentation? Here is the doc link: https://wso2.github.io/oxygen-ui/?path=/docs/surfaces-accordion--docs |
chamals3n4
force-pushed
the
fix/678-span-collapse
branch
from
9374c92 to
64ce77a
Compare
Author
|
hi @rasika2012 , i updated it to use the oxygen ui accordion Screencast.from.2026-06-18.15-51-34.webm |
rasika2012
requested changes
Jun 22, 2026
Co-authored-by: Rasika Maduranga <rasikat77@gmail.com>
…Overview.tsx Co-authored-by: Rasika Maduranga <rasikat77@gmail.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Purpose
resolves #678
Goals
Add expand/collapse toggle for input and output messages in the span overview section to reduce UI clutter when spans contain large payloads.
Approach
add a useState(false) hook to the MessageList component to track the expanded/collapsed state. wrapped the messages list in MUI Collapse component, and replaced the static section header with a clickable Box that toggles the state on click. and add ChevronDown/ChevronUp icons from oxygen ui icons to visually indicate the current state. by default bothi input and output messages sections are collapsed .
Screencast.from.2026-06-15.14-02-29.webm
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit