-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat(ui): adjust roo font size #8457
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.
I found some issues that need attention. See inline comments.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
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.
I found some issues that need attention. See inline comments.
|
Very nice @elianiva . My old eyes thank you for this! |
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.
I found some issues that need attention. See inline comments.
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.
I found some issues that need attention. See inline comments.
Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>
…oo-Code into feat/adjust-font-size
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.
I found some issues that need attention. See inline comments.
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.
I found some issues that need attention. See inline comments.
| word-break: ${({ wordwrap }) => (wordwrap === "false" ? "normal" : "normal")}; | ||
| overflow-wrap: ${({ wordwrap }) => (wordwrap === "false" ? "normal" : "break-word")}; | ||
| font-size: 0.95em; | ||
| font-size: calc(0.95em * var(--roo-font-size-multiplier, 1)); |
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.
P1: Double-applied font scaling for code blocks. Styled containers already scale text via --roo-font-size-multiplier; multiplying an em-based size by the multiplier here applies it twice when nested (e.g., inside MarkdownBlock). Use a single scale source.
| font-size: calc(0.95em * var(--roo-font-size-multiplier, 1)); | |
| font-size: 0.95em; |
| code:not(pre > code) { | ||
| font-family: var(--vscode-editor-font-family, monospace); | ||
| font-size: 0.85em; | ||
| font-size: calc(0.85em * var(--roo-font-size-multiplier, 1)); |
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.
P1: Double-applied font scaling for inline code. This element sits under a container that already sets font-size from var(--vscode-font-size) * --roo-font-size-multiplier, so multiplying the em size again compounds the factor. Remove the extra multiplier.
| font-size: calc(0.85em * var(--roo-font-size-multiplier, 1)); | |
| font-size: 0.85em; |
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.
I found some issues that need attention. See inline comments.
| code:not(pre > code) { | ||
| font-family: var(--vscode-editor-font-family, monospace); | ||
| font-size: 0.85em; | ||
| font-size: calc(0.85em * var(--roo-font-size-multiplier, 1)); |
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.
P1: Double-applied font scaling. Inline code uses em relative to its parent which already includes --roo-font-size-multiplier via StyledMarkdown. Multiplying by the multiplier again compounds scaling. Remove the extra multiplier so it scales once consistently.
| font-size: calc(0.85em * var(--roo-font-size-multiplier, 1)); | |
| font-size: 0.85em; |
| /* KaTeX styling */ | ||
| .katex { | ||
| font-size: 1.1em; | ||
| font-size: calc(1.1em * var(--roo-font-size-multiplier, 1)); |
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.
P2: Double-applied font scaling for KaTeX. This uses em (relative to parent that already applies the multiplier) and multiplies again. Keep it proportional to parent without the extra multiplier.
| font-size: calc(1.1em * var(--roo-font-size-multiplier, 1)); | |
| font-size: 1.1em; |
| word-break: ${({ wordwrap }) => (wordwrap === "false" ? "normal" : "normal")}; | ||
| overflow-wrap: ${({ wordwrap }) => (wordwrap === "false" ? "normal" : "break-word")}; | ||
| font-size: 0.95em; | ||
| font-size: calc(0.95em * var(--roo-font-size-multiplier, 1)); |
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.
P2: Double-applied font scaling in code/pre. The parent text already reflects --roo-font-size-multiplier. Using em here and multiplying by the multiplier again compounds scaling. Remove the multiplier to avoid double scaling.
| font-size: calc(0.95em * var(--roo-font-size-multiplier, 1)); | |
| font-size: 0.95em; |
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.
I found some issues that need attention. See inline comments.
|
I just realised that I need to replace literally all reference to VSCodeCheckbox because it didn't get affected, I only changed the one in provider settings page. Although, I'll wait for approval whether or not we want to change the whole appearance or just changing the chat UI before doing that. |
|
@brunobergher thoughts? |
|
Sorry for the delay, @elianiva, this is awesome! I agree we need to be more consistent in using font-size as a basis for scale everywhere, but this already improves things a lot. My only 3 comments are:
This is create work in any case, I really appreciate it. |
Related GitHub Issue
Closes: #8100
Roo Code Task Context (Optional)
Description
This PR adds the ability to scale the font size inside Roo's window. I didn't go with small, medium, large and instead went with multiplier because it gives the user more flexibility.
I didn't add mouse wheel binding or keyboard binding by default because it's already used by the editor and I prefer to leave that choice to the users.
We now have these two new commands:
Along with a configurable setting using
roo-cline.fontSizeMultiplierthat the users can configure themselves.As a side effect, I've also replaced a few components due to them being a web components, which prevents me from updating the font-size because they're using shadow DOM. This change shouldn't matter that much, it only adds consistency to the UI.
Test Procedure
Change the settings or use the provided command from the command palette and observe the change. The only part that changes is the font size.
Pre-Submission Checklist
Screenshots / Videos
Normal size:
Smol size:
Comically large size:
Documentation Updates
Additional Notes
While working on this I also found that we used different ways of stylings. Might worth to revisit that in the future for consistencies.
Also, the issue didn't explain the scope on which section should have the size changed. I'm not sure if we only want to change the font in the chat? I think that's what we want because the rest can be changed using vscode settings, but for now I went with changing everything for consistency.
Let me know which one is the correct one and I'll update this PR :)
Get in Touch
@elianiva
Important
Adds font size adjustment feature using a multiplier in Roo UI, with new commands and updates across components and styles.
roo-cline.fontSizeMultiplierinregisterCommands.ts.increaseFontSizeanddecreaseFontSizeinvscode.ts.ClineProvider.tsto handle font size changes.App.tsx,BrowserSessionRow.tsx,ChatRow.tsx, and other components to use--roo-font-size-multiplier.VSCodeCheckboxwithCheckboxin multiple settings components.index.cssandpreflight.cssto incorporate--roo-font-size-multiplierfor consistent font scaling.button.tsxandstyles.tsfor consistent UI appearance with new font size settings.ExtensionStateContext.tsxto includefontSizeMultiplierin state.This description was created by
for a9da56d. You can customize this summary. It will automatically update as commits are pushed.