-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add interface text size setting for Roo UI #8103
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1558,6 +1558,10 @@ export const webviewMessageHandler = async ( | |
| await updateGlobalState("language", message.text as Language) | ||
| await provider.postStateToWebview() | ||
| break | ||
| case "interfaceTextSize": | ||
| await updateGlobalState("interfaceTextSize", message.text as "small" | "medium" | "large" | undefined) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This message handler case should have test coverage. Could we add a test to verify the state update and webview posting work correctly? |
||
| await provider.postStateToWebview() | ||
| break | ||
| case "openRouterImageApiKey": | ||
| await provider.contextProxy.setValue("openRouterImageApiKey", message.text) | ||
| await provider.postStateToWebview() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,7 @@ const App = () => { | |
| cloudApiUrl, | ||
| renderContext, | ||
| mdmCompliant, | ||
| interfaceTextSize, | ||
| } = useExtensionState() | ||
|
|
||
| // Create a persistent state manager | ||
|
|
@@ -245,7 +246,7 @@ const App = () => { | |
|
|
||
| // Do not conditionally load ChatView, it's expensive and there's state we | ||
| // don't want to lose (user input, disableInput, askResponse promise, etc.) | ||
| return showWelcome ? ( | ||
| const content = showWelcome ? ( | ||
| <WelcomeView /> | ||
| ) : ( | ||
| <> | ||
|
|
@@ -345,6 +346,9 @@ const App = () => { | |
| )} | ||
| </> | ||
| ) | ||
|
|
||
| // Wrap content with interface text size class | ||
| return <div className={`interface-text-${interfaceTextSize || "medium"}`}>{content}</div> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default value 'medium' is hardcoded here. Consider extracting this to a constants file to maintain consistency across the codebase. |
||
| } | ||
|
|
||
| const queryClient = new QueryClient() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import React from "react" | ||
| import { Type } from "lucide-react" | ||
| import { VSCodeDropdown, VSCodeOption } from "@vscode/webview-ui-toolkit/react" | ||
|
|
||
| import { useAppTranslation } from "@src/i18n/TranslationContext" | ||
| import { SetCachedStateField } from "./types" | ||
| import { SectionHeader } from "./SectionHeader" | ||
| import { Section } from "./Section" | ||
|
|
||
| interface InterfaceSettingsProps { | ||
| interfaceTextSize?: "small" | "medium" | "large" | ||
| setCachedStateField: SetCachedStateField<"interfaceTextSize"> | ||
| } | ||
|
|
||
| export const InterfaceSettings = ({ interfaceTextSize = "medium", setCachedStateField }: InterfaceSettingsProps) => { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This component needs test coverage. Could we add tests to verify the dropdown behavior and state updates work correctly? |
||
| const { t } = useAppTranslation() | ||
|
|
||
| return ( | ||
| <div> | ||
| <SectionHeader> | ||
| <div className="flex items-center gap-2"> | ||
| <Type className="w-4" /> | ||
| <div>{t("settings:interface.title")}</div> | ||
| </div> | ||
| </SectionHeader> | ||
|
|
||
| <Section> | ||
| <div className="dropdown-container"> | ||
| <label htmlFor="interface-text-size">{t("settings:interface.textSize.label")}</label> | ||
| <VSCodeDropdown | ||
| id="interface-text-size" | ||
| value={interfaceTextSize} | ||
| onChange={(e) => { | ||
| const value = (e.target as HTMLSelectElement).value as "small" | "medium" | "large" | ||
| setCachedStateField("interfaceTextSize", value) | ||
| }}> | ||
| <VSCodeOption value="small">{t("settings:interface.textSize.small")}</VSCodeOption> | ||
| <VSCodeOption value="medium">{t("settings:interface.textSize.medium")}</VSCodeOption> | ||
| <VSCodeOption value="large">{t("settings:interface.textSize.large")}</VSCodeOption> | ||
| </VSCodeDropdown> | ||
| <p className="text-xs text-vscode-descriptionForeground mt-1"> | ||
| {t("settings:interface.textSize.description")} | ||
| </p> | ||
| </div> | ||
| </Section> | ||
| </div> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -155,6 +155,8 @@ export interface ExtensionStateContextType extends ExtensionState { | |
| setMaxDiagnosticMessages: (value: number) => void | ||
| includeTaskHistoryInEnhance?: boolean | ||
| setIncludeTaskHistoryInEnhance: (value: boolean) => void | ||
| interfaceTextSize?: "small" | "medium" | "large" | ||
| setInterfaceTextSize: (value: "small" | "medium" | "large") => void | ||
| } | ||
|
|
||
| export const ExtensionStateContext = createContext<ExtensionStateContextType | undefined>(undefined) | ||
|
|
@@ -280,6 +282,7 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode | |
| global: {}, | ||
| }) | ||
| const [includeTaskHistoryInEnhance, setIncludeTaskHistoryInEnhance] = useState(true) | ||
| const [interfaceTextSize, setInterfaceTextSize] = useState<"small" | "medium" | "large">("medium") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another hardcoded 'medium' default. This should use the same constant suggested earlier for consistency. |
||
|
|
||
| const setListApiConfigMeta = useCallback( | ||
| (value: ProviderSettingsEntry[]) => setState((prevState) => ({ ...prevState, listApiConfigMeta: value })), | ||
|
|
@@ -317,6 +320,10 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode | |
| if ((newState as any).includeTaskHistoryInEnhance !== undefined) { | ||
| setIncludeTaskHistoryInEnhance((newState as any).includeTaskHistoryInEnhance) | ||
| } | ||
| // Update interfaceTextSize if present in state message | ||
| if ((newState as any).interfaceTextSize !== undefined) { | ||
| setInterfaceTextSize((newState as any).interfaceTextSize) | ||
| } | ||
| // Handle marketplace data if present in state message | ||
| if (newState.marketplaceItems !== undefined) { | ||
| setMarketplaceItems(newState.marketplaceItems) | ||
|
|
@@ -538,6 +545,8 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode | |
| }, | ||
| includeTaskHistoryInEnhance, | ||
| setIncludeTaskHistoryInEnhance, | ||
| interfaceTextSize, | ||
| setInterfaceTextSize, | ||
| } | ||
|
|
||
| return <ExtensionStateContext.Provider value={contextValue}>{children}</ExtensionStateContext.Provider> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |
| @theme { | ||
| --font-display: var(--vscode-font-family); | ||
|
|
||
| /* Default text sizes (medium) */ | ||
| --text-xs: calc(var(--vscode-font-size) * 0.85); | ||
| --text-sm: calc(var(--vscode-font-size) * 0.9); | ||
| --text-base: var(--vscode-font-size); | ||
|
|
@@ -490,3 +491,22 @@ input[cmdk-input]:focus { | |
| transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1); | ||
| transition-duration: 150ms; | ||
| } | ||
|
|
||
| /* Interface text size classes */ | ||
| .interface-text-small { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These CSS variables affect all text globally within the wrapper. Have you considered potential conflicts with other UI elements that might need different scaling? Perhaps we could use more specific CSS custom properties or add additional classes for finer control. |
||
| --text-xs: calc(var(--vscode-font-size) * 0.75); | ||
| --text-sm: calc(var(--vscode-font-size) * 0.8); | ||
| --text-base: calc(var(--vscode-font-size) * 0.85); | ||
| --text-lg: calc(var(--vscode-font-size) * 0.95); | ||
| } | ||
|
|
||
| .interface-text-medium { | ||
| /* Default sizes - already defined above */ | ||
| } | ||
|
|
||
| .interface-text-large { | ||
| --text-xs: calc(var(--vscode-font-size) * 0.95); | ||
| --text-sm: calc(var(--vscode-font-size) * 1); | ||
| --text-base: calc(var(--vscode-font-size) * 1.15); | ||
| --text-lg: calc(var(--vscode-font-size) * 1.25); | ||
| } | ||
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.
Good use of Zod enum for type safety! However, consider creating a dedicated TypeScript enum or const assertion that can be imported and used throughout the codebase to avoid string literals.