feat(ui): add chat font size multiplier setting#10489
feat(ui): add chat font size multiplier setting#10489roomote[bot] wants to merge 4 commits intomainfrom
Conversation
Review complete. Found 1 issue (the previous floating point issue has been fixed):
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
src/activate/registerCommands.ts
Outdated
| const currentMultiplier = visibleProvider.contextProxy.getValue("chatFontSizeMultiplier") ?? 1 | ||
| const newMultiplier = Math.min(2, currentMultiplier + 0.1) |
There was a problem hiding this comment.
Floating point precision issue: using currentMultiplier + 0.1 can produce values like 1.2000000000000002 after repeated increments, which will display in the UI input field. Consider rounding to one decimal place:
| const currentMultiplier = visibleProvider.contextProxy.getValue("chatFontSizeMultiplier") ?? 1 | |
| const newMultiplier = Math.min(2, currentMultiplier + 0.1) | |
| const currentMultiplier = visibleProvider.contextProxy.getValue("chatFontSizeMultiplier") ?? 1 | |
| const newMultiplier = Math.min(2, Math.round((currentMultiplier + 0.1) * 10) / 10) |
The same applies to decreaseChatFontSize below.
Fix it with Roo Code or mention @roomote and request a fix.
hannesrudolph
added
the
Issue/PR - Triage
|
Closing because the linked issue is being closed. |
|
Hi @hannesrudolph / @roomote – the PR was closed but not merged. Could you please elaborate on why?. I could not find any PR or Merge that actually enabled Font Size adjustment for roo anywhere. Thanks |
Fixed floating point precision in font size increment/decrement commands by rounding to 1 decimal place. All local checks passed. |
- Add chatFontSizeMultiplier to global settings with min 0.5 max 2 bounds - Add commands: increaseChatFontSize, decreaseChatFontSize, resetChatFontSize - Add UI Settings control with number input and reset button - Apply font size multiplier only to ChatView container (scoped per Bruno feedback) - Add English translations for the new setting - Update UISettings tests Addresses issue #8100 per feedback from Bruno in PR #8457
roomote
bot
force-pushed
the
feature/chat-font-size-multiplier
branch
from
1df005a to
007629a
Compare
| openRouterImageGenerationSelectedModel, | ||
| reasoningBlockCollapsed, | ||
| enterBehavior, | ||
| chatFontSizeMultiplier, |
There was a problem hiding this comment.
chatFontSizeMultiplier is destructured from cachedState here but is never included in the updatedSettings payload inside handleSubmit (around line 416, next to enterBehavior). The commands persist correctly via contextProxy.setValue(), but changes made through the Settings UI will be lost on reload because handleSubmit never sends this field to the extension host. Add chatFontSizeMultiplier: chatFontSizeMultiplier ?? 1, to the updatedSettings object in handleSubmit.
Fix it with Roo Code or mention @roomote and request a fix.
|
Code Review Summary - PR #10489 Previous review items (telemetry on intermediate keystrokes, missing test coverage) have been addressed. One new minor issue flagged.
|
- Move telemetry capture from onChange to onBlur in handleFontSizeMultiplierBlur so intermediate keystrokes do not emit events (addresses review comment 3e180acf39) - Add 14 new test cases covering font size input rendering, change/blur/reset handlers, clamping logic, NaN handling, and telemetry behavior (addresses review comment 4bf3aa3bb4)
| const clampedValue = Math.max(0.5, Math.min(2, numValue)) | ||
| setLocalMultiplier(clampedValue.toString()) | ||
|
|
||
| // Track telemetry event on blur to capture only the user's final value | ||
| telemetryClient.capture("ui_settings_chat_font_size_changed", { | ||
| multiplier: clampedValue, | ||
| }) |
There was a problem hiding this comment.
handleFontSizeMultiplierBlur fires ui_settings_chat_font_size_changed telemetry on every blur with a valid number, even when the value hasn't actually changed (e.g., user clicks into the input and tabs away without editing). The event name implies a change occurred, but none did. This inflates the metric and makes it unreliable for measuring real setting changes. Adding a guard if (clampedValue !== chatFontSizeMultiplier) before the capture call would limit it to actual changes.
| const clampedValue = Math.max(0.5, Math.min(2, numValue)) | |
| setLocalMultiplier(clampedValue.toString()) | |
| // Track telemetry event on blur to capture only the user's final value | |
| telemetryClient.capture("ui_settings_chat_font_size_changed", { | |
| multiplier: clampedValue, | |
| }) | |
| const clampedValue = Math.max(0.5, Math.min(2, numValue)) | |
| setLocalMultiplier(clampedValue.toString()) | |
| // Only fire telemetry if the value actually changed | |
| if (clampedValue !== chatFontSizeMultiplier) { | |
| telemetryClient.capture("ui_settings_chat_font_size_changed", { | |
| multiplier: clampedValue, | |
| }) | |
| } |
Fix it with Roo Code or mention @roomote and request a fix.
3 participants
Related GitHub Issue
Closes: #8100
Roo Code Task Context (Optional)
This PR was implemented by @roomote per the request in issue #8100 based on feedback from Bruno in PR #8457.
Description
This PR attempts to address Issue #8100 by implementing a chat font size multiplier feature. Feedback and guidance are welcome.
Key implementation details:
roo-cline.increaseChatFontSize- increases by 0.1 (capped at 2)roo-cline.decreaseChatFontSize- decreases by 0.1 (capped at 0.5)roo-cline.resetChatFontSize- resets to default value of 1Test Procedure
Manual testing:
Command testing:
Automated tests:
cd webview-ui && npx vitest run src/components/settings/__tests__/UISettings.spec.tsxPre-Submission Checklist
Screenshots / Videos
N/A - UI changes are minimal (number input and reset button in settings)
Documentation Updates
Additional Notes
This implementation follows Bruno's specific feedback from PR #8457:
Get in Touch
@roomote (automated assistant)
Interactively review PR in Roo Code Cloud