Skip to content

ux: improve responsiveness on small widths for auto-approve button #8152

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

ux: improve responsiveness on small widths for auto-approve button #8152

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b48385
ux: improve responsiveness on small widths for auto-approve button
Sep 18, 2025
0ba2389
i18n: add short off label for trigger in multiple languages
hannesrudolph Oct 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions webview-ui/src/components/chat/AutoApproveDropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,24 +173,35 @@ export const AutoApproveDropdown = ({ disabled = false, triggerClassName = "" }:
"inline-flex items-center gap-1.5 relative whitespace-nowrap px-1.5 py-1 text-xs",
"bg-transparent border border-[rgba(255,255,255,0.08)] rounded-md text-vscode-foreground",
"transition-all duration-150 focus:outline-none focus-visible:ring-1 focus-visible:ring-vscode-focusBorder focus-visible:ring-inset",
"max-[400px]:shrink-0",
Copy link

@roomote roomote bot Sep 18, 2025

Choose a reason for hiding this comment

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

The 400px breakpoint is hardcoded in multiple places (lines 176, 182, 183, 191, 198). Consider extracting this to a constant or CSS variable for better maintainability. For example:

const NARROW_SCREEN_BREAKPOINT = '400px';
// Then use: `max-[${NARROW_SCREEN_BREAKPOINT}]` in template literals

Or define it as a Tailwind config value if the project supports it.

disabled
? "opacity-50 cursor-not-allowed"
: "opacity-90 hover:opacity-100 hover:bg-[rgba(255,255,255,0.03)] hover:border-[rgba(255,255,255,0.15)] cursor-pointer",
triggerClassName,
)}>
{!effectiveAutoApprovalEnabled ? (
<X className="size-3 flex-shrink-0" />
) : (
<CheckCheck className="size-3 flex-shrink-0" />
)}
<CheckCheck className="min-[400px]:hidden size-3 flex-shrink-0" />
Copy link

@roomote roomote bot Sep 18, 2025

Choose a reason for hiding this comment

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

Is this intentional? The button always shows the CheckCheck icon on narrow screens regardless of the auto-approve state. This could be confusing since users expect the icon to reflect whether auto-approve is on or off. Consider keeping the state-based icon logic even on narrow screens:

Suggested change
<CheckCheck className="min-[400px]:hidden size-3 flex-shrink-0" />
<span className="min-[400px]:hidden">
{!effectiveAutoApprovalEnabled ? (
<X className="size-3 flex-shrink-0" />
) : (
<CheckCheck className="size-3 flex-shrink-0" />
)}
</span>

Copy link
Author

@VeryEvilHumna VeryEvilHumna Sep 18, 2025

Choose a reason for hiding this comment

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

That is intentional, it keeps the semantic meaning of the button (double checkmark == auto-approve) even without the text on the button that says what it does

<span className="max-[400px]:hidden">
{!effectiveAutoApprovalEnabled ? (
<X className="size-3 flex-shrink-0" />
) : (
<CheckCheck className="size-3 flex-shrink-0" />
)}
</span>

<span className="truncate min-w-0">
<span className="min-[400px]:inline hidden truncate min-w-0">
{!effectiveAutoApprovalEnabled
? t("chat:autoApprove.triggerLabelOff")
: enabledCount === totalCount
? t("chat:autoApprove.triggerLabelAll")
: t("chat:autoApprove.triggerLabel", { count: enabledCount })}
</span>
<span className="min-[400px]:hidden min-w-0">
{!effectiveAutoApprovalEnabled
? t("chat:autoApprove.triggerLabelOffShort")
: enabledCount === totalCount
? t("chat:autoApprove.triggerLabelAll")
: enabledCount}
</span>
</PopoverTrigger>
</StandardTooltip>
<PopoverContent
Expand Down
3 changes: 2 additions & 1 deletion webview-ui/src/i18n/locales/en/chat.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,8 @@
"triggerLabel_zero": "0 auto-approve",
"triggerLabel_one": "1 auto-approved",
"triggerLabel_other": "{{count}} auto-approved",
"triggerLabelAll": "YOLO"
"triggerLabelAll": "YOLO",
"triggerLabelOffShort": "Off"
Copy link

@roomote roomote bot Sep 18, 2025

Choose a reason for hiding this comment

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

Good addition! However, this new translation key needs to be added to all other locale files to prevent missing translation warnings. The following locales are missing this key:

  • Spanish (es)
  • French (fr)
  • German (de)
  • Chinese (zh-CN)
  • Japanese (ja)
  • And all other supported locales

Even if you don't know the translations, please add the key with the English value as a placeholder to prevent runtime warnings.

},
"announcement": {
"title": "🎉 Roo Code {{version}} Released",
Expand Down
3 changes: 2 additions & 1 deletion webview-ui/src/i18n/locales/ru/chat.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading