Skip to content

Commit ea0c94a

Browse files
hannesrudolphdaniel-lxs
authored andcommitted
feat: improve unlimited diagnostics UI with integrated slider design
- Replace separate unlimited checkbox with intuitive slider design - Slider maximum position (100) now represents 'unlimited' value - Internal value of -1 is set when slider is at maximum position - Display 'Unlimited' text when slider is at max position - Update all 18 language translations for the new UI text - Update tests to reflect the new integrated design approach This provides a cleaner, more intuitive interface for setting diagnostic limits without the need for a separate checkbox control.
1 parent 93860c7 commit ea0c94a

File tree

20 files changed

+120
-52
lines changed

20 files changed

+120
-52
lines changed

webview-ui/src/components/settings/ContextManagementSettings.tsx

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,22 @@ export const ContextManagementSettings = ({
226226
min={1}
227227
max={100}
228228
step={1}
229-
value={[maxDiagnosticMessages ?? 50]}
230-
onValueChange={([value]) => setCachedStateField("maxDiagnosticMessages", value)}
229+
value={[
230+
maxDiagnosticMessages !== undefined && maxDiagnosticMessages <= 0
231+
? 100
232+
: (maxDiagnosticMessages ?? 50),
233+
]}
234+
onValueChange={([value]) => {
235+
// When slider reaches 100, set to -1 (unlimited)
236+
setCachedStateField("maxDiagnosticMessages", value === 100 ? -1 : value)
237+
}}
231238
data-testid="max-diagnostic-messages-slider"
232-
disabled={maxDiagnosticMessages === -1}
233239
/>
234-
<span className="w-10">
235-
{maxDiagnosticMessages === -1 ? "∞" : (maxDiagnosticMessages ?? 50)}
240+
<span className="w-20 text-sm font-medium">
241+
{(maxDiagnosticMessages !== undefined && maxDiagnosticMessages <= 0) ||
242+
maxDiagnosticMessages === 100
243+
? t("settings:contextManagement.diagnostics.maxMessages.unlimitedLabel") || "Unlimited"
244+
: (maxDiagnosticMessages ?? 50)}
236245
</span>
237246
<Button
238247
variant="ghost"
@@ -244,18 +253,12 @@ export const ContextManagementSettings = ({
244253
<span className="codicon codicon-discard" />
245254
</Button>
246255
</div>
247-
<div className="flex items-center gap-2 mt-2">
248-
<VSCodeCheckbox
249-
checked={maxDiagnosticMessages === -1}
250-
onChange={(e: any) =>
251-
setCachedStateField("maxDiagnosticMessages", e.target.checked ? -1 : 50)
252-
}
253-
data-testid="max-diagnostic-messages-unlimited-checkbox">
254-
{t("settings:contextManagement.diagnostics.maxMessages.unlimited")}
255-
</VSCodeCheckbox>
256-
</div>
257256
<div className="text-vscode-descriptionForeground text-sm mt-1">
258-
{t("settings:contextManagement.diagnostics.maxMessages.description")}
257+
{(maxDiagnosticMessages !== undefined && maxDiagnosticMessages <= 0) ||
258+
maxDiagnosticMessages === 100
259+
? t("settings:contextManagement.diagnostics.maxMessages.unlimitedDescription") ||
260+
"All diagnostic messages will be included. Use with caution as this may significantly increase token usage."
261+
: t("settings:contextManagement.diagnostics.maxMessages.description")}
259262
</div>
260263
</div>
261264
</Section>

webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,19 @@
33
import { render, screen, fireEvent, waitFor } from "@/utils/test-utils"
44
import { ContextManagementSettings } from "../ContextManagementSettings"
55

6+
// Mock the translation hook
7+
vi.mock("@/hooks/useAppTranslation", () => ({
8+
useAppTranslation: () => ({
9+
t: (key: string) => {
10+
// Return specific translations for our test cases
11+
if (key === "settings:contextManagement.diagnostics.maxMessages.unlimitedLabel") {
12+
return "Unlimited"
13+
}
14+
return key
15+
},
16+
}),
17+
}))
18+
619
// Mock the UI components
720
vi.mock("@/components/ui", () => ({
821
...vi.importActual("@/components/ui"),
@@ -131,7 +144,7 @@ describe("ContextManagementSettings", () => {
131144
fireEvent.change(slider, { target: { value: "100" } })
132145

133146
await waitFor(() => {
134-
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", 100)
147+
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", -1)
135148
})
136149
})
137150

@@ -151,9 +164,17 @@ describe("ContextManagementSettings", () => {
151164

152165
expect(screen.getByText("25")).toBeInTheDocument()
153166

154-
// Update value
167+
// Update value - 100 should display as "Unlimited"
155168
rerender(<ContextManagementSettings {...defaultProps} maxDiagnosticMessages={100} />)
156-
expect(screen.getByText("100")).toBeInTheDocument()
169+
expect(
170+
screen.getByText("settings:contextManagement.diagnostics.maxMessages.unlimitedLabel"),
171+
).toBeInTheDocument()
172+
173+
// Test unlimited value (-1) displays as "Unlimited"
174+
rerender(<ContextManagementSettings {...defaultProps} maxDiagnosticMessages={-1} />)
175+
expect(
176+
screen.getByText("settings:contextManagement.diagnostics.maxMessages.unlimitedLabel"),
177+
).toBeInTheDocument()
157178
})
158179

159180
it("renders other context management settings", () => {
@@ -170,7 +191,7 @@ describe("ContextManagementSettings", () => {
170191
})
171192

172193
describe("Edge cases for maxDiagnosticMessages", () => {
173-
it("handles zero value correctly", async () => {
194+
it("handles zero value as unlimited", async () => {
174195
const setCachedStateField = vi.fn()
175196
render(
176197
<ContextManagementSettings
@@ -180,13 +201,17 @@ describe("ContextManagementSettings", () => {
180201
/>,
181202
)
182203

183-
expect(screen.getByText("0")).toBeInTheDocument()
204+
// Zero is now treated as unlimited
205+
expect(
206+
screen.getByText("settings:contextManagement.diagnostics.maxMessages.unlimitedLabel"),
207+
).toBeInTheDocument()
184208

185209
const slider = screen.getByTestId("max-diagnostic-messages-slider")
186-
expect(slider).toHaveValue("0")
210+
// Zero should map to slider position 100 (unlimited)
211+
expect(slider).toHaveValue("100")
187212
})
188213

189-
it("handles negative values by displaying them", async () => {
214+
it("handles negative values as unlimited", async () => {
190215
const setCachedStateField = vi.fn()
191216
render(
192217
<ContextManagementSettings
@@ -196,11 +221,14 @@ describe("ContextManagementSettings", () => {
196221
/>,
197222
)
198223

199-
// Component displays the actual negative value in the text span
200-
expect(screen.getByText("-10")).toBeInTheDocument()
224+
// Component displays "Unlimited" for any negative value
225+
expect(
226+
screen.getByText("settings:contextManagement.diagnostics.maxMessages.unlimitedLabel"),
227+
).toBeInTheDocument()
201228

202-
// Note: The actual slider behavior with negative values depends on the implementation
203-
// In this case, we're just verifying the component renders without errors
229+
// Slider should be at max position (100) for negative values
230+
const slider = screen.getByTestId("max-diagnostic-messages-slider")
231+
expect(slider).toHaveValue("100")
204232
})
205233

206234
it("handles very large numbers by capping at maximum", async () => {
@@ -232,32 +260,33 @@ describe("ContextManagementSettings", () => {
232260
fireEvent.change(slider, { target: { value: "150" } })
233261

234262
await waitFor(() => {
235-
// Should be capped at 100 (the slider's max)
236-
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", 100)
263+
// Should be capped at 100, which maps to -1 (unlimited)
264+
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", -1)
237265
})
238266
})
239267

240-
it("handles boundary value at minimum (0)", async () => {
268+
it("handles boundary value at minimum (1)", async () => {
241269
const setCachedStateField = vi.fn()
242270
render(<ContextManagementSettings {...defaultProps} setCachedStateField={setCachedStateField} />)
243271

244272
const slider = screen.getByTestId("max-diagnostic-messages-slider")
245-
fireEvent.change(slider, { target: { value: "0" } })
273+
fireEvent.change(slider, { target: { value: "1" } })
246274

247275
await waitFor(() => {
248-
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", 0)
276+
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", 1)
249277
})
250278
})
251279

252-
it("handles boundary value at maximum (100)", async () => {
280+
it("handles boundary value at maximum (100) as unlimited (-1)", async () => {
253281
const setCachedStateField = vi.fn()
254282
render(<ContextManagementSettings {...defaultProps} setCachedStateField={setCachedStateField} />)
255283

256284
const slider = screen.getByTestId("max-diagnostic-messages-slider")
257285
fireEvent.change(slider, { target: { value: "100" } })
258286

259287
await waitFor(() => {
260-
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", 100)
288+
// When slider is at 100, it should set the value to -1 (unlimited)
289+
expect(setCachedStateField).toHaveBeenCalledWith("maxDiagnosticMessages", -1)
261290
})
262291
})
263292

webview-ui/src/i18n/locales/ca/settings.json

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/de/settings.json

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/en/settings.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,9 @@
497497
"label": "Maximum diagnostic messages",
498498
"description": "Maximum number of diagnostic messages to include per file. This limit applies to both automatic inclusion (when checkbox is enabled) and manual @problems mentions. Higher values provide more context but increase token usage.",
499499
"resetTooltip": "Reset to default value (50)",
500-
"unlimited": "Unlimited diagnostic messages"
500+
"unlimited": "Unlimited diagnostic messages",
501+
"unlimitedLabel": "Unlimited",
502+
"unlimitedDescription": "All diagnostic messages will be included. Use with caution as this may significantly increase token usage."
501503
}
502504
},
503505
"condensingThreshold": {

webview-ui/src/i18n/locales/es/settings.json

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/fr/settings.json

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/hi/settings.json

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/id/settings.json

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/it/settings.json

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)