Skip to content

Conversation

@KJ7LNW
Copy link
Contributor

@KJ7LNW KJ7LNW commented May 27, 2025

Blocks #3785 #3689

This refactoring is needed by both #3785 and #3689 as they require the standardized structure and consistent use of the existing HistoryItem interface across history components.

Context

The HistoryView and HistoryPreview components had significant code duplication between compact (preview) and full variants, making maintenance difficult.

This PR introduces TaskItem.tsx and TaskItemHeader.tsx to centralize and standardize the rendering of history entries. TaskItem handles the overall structure for "compact" (Preview) and "full" (HistoryView) variants. TaskItemHeader consolidates all metadata (timestamp, tokens, cost, cache, file size) into a single, consistent line above the task content, enhancing visual clarity and reducing UI clutter.

The net change results in fewer lines, and much better organization:

]$ git diff  origin/main..refactor-history-ui-components --stat webview-ui/src/components/history/HistoryPreview.tsx webview-ui/src/components/history/HistoryView.tsx webview-ui/src/components/history/TaskItem.tsx webview-ui/src/components/history/TaskItemHeader.tsx
 webview-ui/src/components/history/HistoryPreview.tsx |  68 +++------------
 webview-ui/src/components/history/HistoryView.tsx    | 252 +++----------------------------------------------------
 webview-ui/src/components/history/TaskItem.tsx       | 125 +++++++++++++++++++++++++++
 webview-ui/src/components/history/TaskItemHeader.tsx | 117 ++++++++++++++++++++++++++
 4 files changed, 264 insertions(+), 298 deletions(-)

Implementation

  • Created TaskItemHeader component to handle shared metadata display
  • Removed custom TaskItemData interface in favor of HistoryItem type to standardize using existing data structures
  • Unified the TSX structure with conditional styling
  • Simplified button handling and workspace display logic

UI

Please notice: The intention of this pull request it is to have absolutely no behavior change.

Font sizes have not changed: Visible differences in scale are because my screenshot size differs and Github is scaling funny.

UI component Before After
Preview image image
Full History image image

How to Test

  • Check both compact and full variants in history view
  • Verify metadata display (timestamp, tokens, cost, cache) is consistent
  • Confirm file size only shows in full variant
  • Test selection mode and button interactions work as before

Get in Touch

Discord: KJ7LNW


Important

Refactor TaskItem component to unify compact and full variants, centralizing logic and metadata display, with updated tests to ensure consistent functionality.

  • Components:
    • Introduces TaskItem.tsx to unify compact and full variants of TaskItem.
    • Adds TaskItemHeader.tsx to handle metadata display (timestamp, tokens, cost, cache, file size).
  • Refactoring:
    • Removes code duplication by centralizing logic in TaskItem and TaskItemHeader.
    • Uses HistoryItem type instead of custom TaskItemData interface.
  • UI Changes:
    • Minor UI adjustments for a more compact display.
    • Consistent metadata display across variants.
  • Testing:
    • Updates tests in HistoryView.test.tsx and adds TaskItem.test.tsx to cover new component structure.

This description was created by Ellipsis for 23b2a8f. You can customize this summary. It will automatically update as commits are pushed.

Eric Wheeler added 2 commits May 26, 2025 20:11
Introduces `TaskItem.tsx` and `TaskItemHeader.tsx` to centralize and
standardize the rendering of history entries. `TaskItem` handles the
overall structure for "compact" (Preview) and "full" (HistoryView)
variants. `TaskItemHeader` consolidates all metadata (timestamp, tokens,
cost, cache, file size) into a single, consistent line above the task
content, enhancing visual clarity and reducing UI clutter.

This refactor significantly simplifies `HistoryPreview.tsx` and
`HistoryView.tsx`. Approximately 314 lines of previous rendering logic
were removed from these components and replaced by 242 lines in the new,
focused, and reusable `TaskItem` and `TaskItemHeader` components,
resulting in a net reduction and improved maintainability.

Most importantly, rendering logic happens in one place.

Key UI Changes:
- Metadata (timestamp, tokens, cost, cache, file size) now displayed
  inline on a single header row in both variants.
- Removed explicit "Tokens:" and "API Cost:" labels for a cleaner look.
- Action buttons (Copy, Export, Delete) in the full view are now
  aligned with the metadata header.
- File size is displayed in the header for the "full" variant only.
- Workspace information is no longer displayed in the "compact" preview.

Component Changes:
- Created `webview-ui/src/components/history/TaskItem.tsx` (125 lines)
- Created `webview-ui/src/components/history/TaskItemHeader.tsx` (117 lines)
- Modified `webview-ui/src/components/history/HistoryPreview.tsx` (-65 lines, +3 lines)
- Modified `webview-ui/src/components/history/HistoryView.tsx` (-249 lines, +3 lines)
- Uses `HistoryItem` type for standardized data handling.

Fixes: #4018
Signed-off-by: Eric Wheeler <[email protected]>
Fixes test failures that occurred after the major refactoring that introduced
the shared TaskItem component. The original implementation was correct but
the tests needed updates to match the new component structure.

- Add data-testid attributes to TaskItemHeader for reliable test selection
- Update TaskItem.test.tsx assertions to use new test IDs for tokens/cache
- Fix Checkbox import path in TaskItem.tsx (ui/checkbox vs ui)
- Add missing mocks for lucide-react and Checkbox in HistoryView.test.tsx
- Update HistoryView test assertions to use correct selectors
- Ensure all 19 history component tests pass successfully

The refactoring reduced code duplication by ~250+ lines while maintaining
functionality, and these test fixes ensure the quality gates remain intact.

Signed-off-by: Eric Wheeler <[email protected]>
@KJ7LNW KJ7LNW force-pushed the refactor-history-ui-components branch from d208854 to 23b2a8f Compare May 27, 2025 03:17
@KJ7LNW KJ7LNW marked this pull request as ready for review May 27, 2025 03:21
@KJ7LNW KJ7LNW requested review from cte and mrubens as code owners May 27, 2025 03:21
@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented May 27, 2025

@daniel-lxs - This PR blocks further progress on #3785 and #3689. I am not sure how you are managing pull request dependency-order, so FYI in case this is useful.

This is ready for review, merging soon would be appreciated so I can continue on the other fronts.

@KJ7LNW KJ7LNW self-assigned this May 27, 2025
@KJ7LNW KJ7LNW changed the title refactor: unify TaskItem component with single implementation refactor: unify HistoryView and HistoryPreview with TaskItem to consistently use the existing HistoryItem interface May 27, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap May 27, 2025
@hannesrudolph
Copy link
Collaborator

@KJ7LNW Is there a way to make this change without making the change to the UI?

@hannesrudolph hannesrudolph moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap May 28, 2025
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label May 30, 2025
Create a new TaskItemFooter component that displays token and cost information
with different styles for compact and full views. Move the CopyButton and
ExportButton from header to footer in full view. Adjust file size display
positioning in the header for better visual alignment.

Signed-off-by: Eric Wheeler <[email protected]>
@vercel
Copy link

vercel bot commented May 30, 2025

Someone is attempting to deploy a commit to the Roo Code Team on Vercel.

A member of the Team first needs to authorize it.

Update data-testid attributes in HistoryView and TaskItem tests to match the
actual implementation in TaskItemFooter component. The tests were looking for
generic "tokens-in" and "tokens-out" attributes, but the implementation uses
variant-specific attributes like "tokens-in-footer-full".

Also added mocks for CopyButton and ExportButton components to resolve
"Element type is invalid" errors during test rendering.

Signed-off-by: Eric Wheeler <[email protected]>
@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented May 30, 2025

@hannesrudolph

your changes have been applied so this is ready

I'm not sure what the Vercel check does, so hopefully it is not relevant.

@hannesrudolph hannesrudolph moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap May 30, 2025
<i className="codicon codicon-arrow-right" style={metadataIconWithTextAdjustStyle} />
<span data-testid="cache-reads">{formatLargeNumber(item.cacheReads || 0)}</span>
</span>
)}
Copy link
Member

@daniel-lxs daniel-lxs May 30, 2025

Choose a reason for hiding this comment

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

Hey @KJ7LNW, I noticed your recent changes are basically moving all the cost/token data back to the bottom.

It seems like the cache information is still on the header next to the timestamp. Is this intentional or was this supposed to be moved as well?

Copy link
Contributor Author

@KJ7LNW KJ7LNW May 30, 2025

Choose a reason for hiding this comment

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

I would love to revert that recent change, but @hannesrudolph was concerned that this would not be accepted if the user interface changed and encouraged me to make it look the same at the cost of slightly more code.

The most recent change creates a <TaskItemFooter> and move things around to look like it did originally, while still maintaining the necessary component organization.

Standardizing all the information on a nice compact header would be great because it shrinks the source code and makes everything consistent, but I am not a user interface person, and the priority for this merge is code organization, not appearance.

This is what it looked like before the change from last night: the left side is current Roo, the right side is what it looked like before I added last night's commits for Hannes:

image

Either version is fine for me, they both provide the necessary factor. However, the computer scientist in me likes the original because the refactor is much cleaner and the components are standardized.

Please merge this PR in whatever form is most likely to be accepted.

Thanks!

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap May 30, 2025
@hannesrudolph
Copy link
Collaborator

@KJ7LNW why is this photo nowhere to be found in this thread?
image

@KJ7LNW
Copy link
Contributor Author

KJ7LNW commented May 30, 2025

@KJ7LNW why is this photo nowhere to be found in this thread?

See OP, I updated original screen shots last night from our change together, so what it shows at the top is the current PR state.

@hannesrudolph
Copy link
Collaborator

@KJ7LNW and what about the placement of cached tokens? I don't see an example with that and it seems that that info would show up in the header not where it currently sits.

@hannesrudolph
Copy link
Collaborator

Closed by #4151

@github-project-automation github-project-automation bot moved this from PR [Changes Requested] to Done in Roo Code Roadmap May 30, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap May 30, 2025
SmartManoj pushed a commit to SmartManoj/Raa-Code that referenced this pull request Jun 13, 2025
…ning. (RooCodeInc#4019)

Fix remaining occurences of protobuf literals.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR - Changes Requested size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants