-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: thinking block designs should occupy lesser space #8685
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 3 files
Prompt for AI agents (all 2 issues)
Understand the root cause of the following 2 issues and fix them.
<file name="gui/src/components/StepContainer/StepContainer.tsx">
<violation number="1" location="gui/src/components/StepContainer/StepContainer.tsx:92">
Passing the current item as prevItem breaks ThinkingBlockPeek’s duplicate-suppression logic, so assistant steps will render an extra thinking block whenever a standalone thinking message precedes them.</violation>
</file>
<file name="gui/src/components/mainInput/belowMainInput/ThinkingBlockPeek.tsx">
<violation number="1" location="gui/src/components/mainInput/belowMainInput/ThinkingBlockPeek.tsx:61">
Replacing the toggle with a plain div drops button semantics and keyboard support, so keyboard users can’t open the thinking block. Please keep this control as a button (or add appropriate semantics and key handling).</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| <ThinkingBlockPeek | ||
| content={props.item.reasoning.text} | ||
| index={props.index} | ||
| prevItem={props.index > 0 ? props.item : null} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing the current item as prevItem breaks ThinkingBlockPeek’s duplicate-suppression logic, so assistant steps will render an extra thinking block whenever a standalone thinking message precedes them.
Prompt for AI agents
Address the following comment on gui/src/components/StepContainer/StepContainer.tsx at line 92:
<comment>Passing the current item as prevItem breaks ThinkingBlockPeek’s duplicate-suppression logic, so assistant steps will render an extra thinking block whenever a standalone thinking message precedes them.</comment>
<file context>
@@ -85,7 +85,14 @@ export default function StepContainer(props: StepContainerProps) {
+ <ThinkingBlockPeek
+ content={props.item.reasoning.text}
+ index={props.index}
+ prevItem={props.index > 0 ? props.item : null}
+ inProgress={!props.item.reasoning?.endAt}
+ />
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will never be true because redactedThinking in props is always passed as undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
| </SpoilerButton> | ||
| <div className="mt-1 flex flex-col px-4"> | ||
| <div className="flex min-w-0 flex-row items-center justify-between gap-2"> | ||
| <div |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replacing the toggle with a plain div drops button semantics and keyboard support, so keyboard users can’t open the thinking block. Please keep this control as a button (or add appropriate semantics and key handling).
Prompt for AI agents
Address the following comment on gui/src/components/mainInput/belowMainInput/ThinkingBlockPeek.tsx at line 61:
<comment>Replacing the toggle with a plain div drops button semantics and keyboard support, so keyboard users can’t open the thinking block. Please keep this control as a button (or add appropriate semantics and key handling).</comment>
<file context>
@@ -66,58 +56,49 @@ function ThinkingBlockPeek({
- </SpoilerButton>
+ <div className="mt-1 flex flex-col px-4">
+ <div className="flex min-w-0 flex-row items-center justify-between gap-2">
+ <div
+ className="text-description flex min-w-0 cursor-pointer flex-row items-center gap-1.5 text-xs transition-colors duration-200 ease-in-out hover:brightness-125"
+ data-testid="thinking-block-peek"
</file context>
✅ Addressed in 302ffb8
Description
Makes thinking blocks occupy less space using the same design as tool calls.
resolves CON-4521
AI Code Review
@continue-reviewChecklist
Screen recording or screenshot
before
after
Tests
[ What tests were added or updated to ensure the changes work as expected? ]
Summary by cubic
Make thinking blocks more compact by matching the simple tool call UI, reducing vertical space and improving readability. Addresses CON-4521.
Written for commit 302ffb8. Summary will update automatically on new commits.