-
Notifications
You must be signed in to change notification settings - Fork 14
fix(app): restore model picker hover and tooltip contrast #453
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| import { test, expect } from "../fixtures" | ||
| import { openPalette } from "../actions" | ||
| import { promptSelector } from "../selectors" | ||
| import type { Page } from "@playwright/test" | ||
|
|
||
| function rgb(value: string) { | ||
| const [r, g, b] = value.match(/\d+/g)?.slice(0, 3).map(Number) ?? [] | ||
| if (r === undefined || g === undefined || b === undefined) throw new Error(`Invalid color: ${value}`) | ||
| return [r, g, b] as const | ||
| } | ||
|
|
||
| function luminance([r, g, b]: readonly [number, number, number]) { | ||
| const channel = (value: number) => { | ||
| const normalized = value / 255 | ||
| return normalized <= 0.03928 ? normalized / 12.92 : ((normalized + 0.055) / 1.055) ** 2.4 | ||
| } | ||
| return 0.2126 * channel(r) + 0.7152 * channel(g) + 0.0722 * channel(b) | ||
| } | ||
|
|
||
| function contrast(a: readonly [number, number, number], b: readonly [number, number, number]) { | ||
| const lighter = Math.max(luminance(a), luminance(b)) | ||
| const darker = Math.min(luminance(a), luminance(b)) | ||
| return (lighter + 0.05) / (darker + 0.05) | ||
| } | ||
|
|
||
| function cssColor(page: Page, variable: string) { | ||
| return page.evaluate((name) => { | ||
| const node = document.createElement("div") | ||
| node.style.color = `var(${name})` | ||
| document.body.appendChild(node) | ||
| const color = getComputedStyle(node).color | ||
| node.remove() | ||
| return color | ||
| }, variable) | ||
| } | ||
|
|
||
| test("model picker hover and tooltip stay visible", async ({ page, gotoSession }) => { | ||
| await gotoSession() | ||
|
|
||
| await page.locator(promptSelector).click() | ||
| await page.keyboard.type("/model") | ||
|
|
||
| const command = page.locator('[data-slash-id="model.choose"]') | ||
| await expect(command).toBeVisible() | ||
| await command.hover() | ||
| await page.keyboard.press("Enter") | ||
|
|
||
| const picker = page.getByRole("dialog") | ||
| await expect(picker).toBeVisible() | ||
| const hoverSurface = await cssColor(page, "--surface-sunken") | ||
|
|
||
| const row = picker.locator('[data-component="list-item"]').first() | ||
| await expect(row).toBeVisible() | ||
|
|
||
| const pickerBackground = await picker.evaluate((node) => getComputedStyle(node).backgroundColor) | ||
| await row.hover() | ||
| await expect(row).toHaveAttribute("data-active", "true") | ||
|
|
||
| const rowBackground = await row.evaluate((node) => getComputedStyle(node).backgroundColor) | ||
| expect(rowBackground).toBe(hoverSurface) | ||
| expect(contrast(rgb(rowBackground), rgb(pickerBackground))).toBeGreaterThan(1.01) | ||
|
|
||
| const tooltip = page.locator('[data-component="tooltip"]') | ||
| await expect(tooltip).toBeVisible() | ||
|
|
||
| const tooltipBackground = await tooltip.evaluate((node) => { | ||
| const style = getComputedStyle(node) | ||
| return style.backgroundColor | ||
| }) | ||
| const textColors = await tooltip.locator("div").evaluateAll((nodes) => { | ||
| return nodes | ||
| .filter((node) => node.textContent?.trim()) | ||
| .map((node) => getComputedStyle(node).color) | ||
| }) | ||
| expect(textColors.length).toBeGreaterThan(0) | ||
| for (const textColor of textColors) { | ||
| expect(contrast(rgb(tooltipBackground), rgb(textColor))).toBeGreaterThanOrEqual(4.5) | ||
| } | ||
| }) | ||
|
|
||
| test("prompt workspace and variant menus keep visible hover states", async ({ page, gotoSession }) => { | ||
| await gotoSession() | ||
| const hoverSurface = await cssColor(page, "--surface-sunken") | ||
|
|
||
| const workspace = page.locator('[data-action="prompt-workspace"]') | ||
| await expect(workspace).toBeVisible() | ||
| await workspace.click() | ||
|
|
||
| const workspaceMenu = page.getByRole("menu").filter({ hasText: /Workspace|工作区|项目/i }) | ||
| await expect(workspaceMenu).toBeVisible() | ||
| const workspaceItem = workspaceMenu.locator('[role="menuitemradio"], [role="menuitem"]').first() | ||
| await expect(workspaceItem).toBeVisible() | ||
|
|
||
| const workspaceBackground = await workspaceMenu.evaluate((node) => getComputedStyle(node.parentElement ?? node).backgroundColor) | ||
| await workspaceItem.hover() | ||
| const workspaceItemBackground = await workspaceItem.evaluate((node) => getComputedStyle(node).backgroundColor) | ||
| expect(workspaceItemBackground).toBe(hoverSurface) | ||
| expect(contrast(rgb(workspaceItemBackground), rgb(workspaceBackground))).toBeGreaterThan(1.01) | ||
|
|
||
| await page.keyboard.press("Escape") | ||
| await expect(workspaceMenu).toHaveCount(0) | ||
|
|
||
| const variant = page.locator('[data-action="prompt-model-variant"]') | ||
| await expect(variant).toBeVisible() | ||
| await variant.click() | ||
|
|
||
| const variantMenu = page.getByRole("menu").filter({ hasText: /Reasoning effort|思考强度/i }) | ||
| await expect(variantMenu).toBeVisible() | ||
| const variantItem = variantMenu.locator('[role="menuitemradio"]').first() | ||
| await expect(variantItem).toBeVisible() | ||
|
|
||
| const variantBackground = await variantMenu.evaluate((node) => getComputedStyle(node.parentElement ?? node).backgroundColor) | ||
| await variantItem.hover() | ||
| const variantItemBackground = await variantItem.evaluate((node) => getComputedStyle(node).backgroundColor) | ||
| expect(variantItemBackground).toBe(hoverSurface) | ||
| expect(contrast(rgb(variantItemBackground), rgb(variantBackground))).toBeGreaterThan(1.01) | ||
| }) | ||
|
|
||
| test("command palette search results keep visible hover states", async ({ page, gotoSession }) => { | ||
| await gotoSession() | ||
| const hoverSurface = await cssColor(page, "--surface-sunken") | ||
|
|
||
| const palette = await openPalette(page) | ||
| await palette.getByRole("textbox").fill("open") | ||
|
|
||
| const row = palette.locator('[data-component="list-item"]').first() | ||
| await expect(row).toBeVisible() | ||
|
|
||
| const paletteBackground = await palette.evaluate((node) => getComputedStyle(node).backgroundColor) | ||
| await row.hover() | ||
| await expect(row).toHaveAttribute("data-active", "true") | ||
|
|
||
| const rowBackground = await row.evaluate((node) => getComputedStyle(node).backgroundColor) | ||
| expect(rowBackground).toBe(hoverSurface) | ||
| expect(contrast(rgb(rowBackground), rgb(paletteBackground))).toBeGreaterThan(1.01) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { describe, expect, test } from "bun:test" | ||
|
|
||
| const root = new URL("../../../../", import.meta.url) | ||
| const read = async (path: string) => Bun.file(new URL(path, root)).text() | ||
|
|
||
| describe("model picker visual regression guard", () => { | ||
| test("model tooltip content inherits the tooltip text color", async () => { | ||
| const source = await read("packages/app/src/components/model-tooltip.tsx") | ||
|
|
||
| expect(source).not.toContain("text-fg-on-brand") | ||
| }) | ||
|
|
||
| test("model list active row uses a visible hover surface", async () => { | ||
| const source = await read("packages/ui/src/components/list.css") | ||
|
|
||
| expect(source).toContain('&[data-active="true"]') | ||
| expect(source).toMatch(/&\[data-active="true"\]\s*\{[\s\S]*?background:\s*var\(--surface-sunken\)/) | ||
| expect(source).not.toMatch(/&\[data-active="true"\]\s*\{[\s\S]*?background:\s*var\(--surface-raised\)/) | ||
| }) | ||
|
|
||
| test("prompt workspace and variant menu rows use a visible hover surface", async () => { | ||
| const workspace = await read("packages/app/src/components/prompt-input/workspace-chip.tsx") | ||
| const promptInput = await read("packages/app/src/components/prompt-input.tsx") | ||
| const select = await read("packages/ui/src/components/select.css") | ||
|
|
||
| expect(workspace).not.toContain("hover:bg-surface-raised") | ||
| expect(workspace).not.toContain("focus-visible:bg-surface-raised") | ||
| expect(promptInput).not.toContain("hover:bg-surface-raised focus-visible:bg-surface-raised") | ||
| expect(select).not.toMatch(/&\[data-highlighted\]\s*\{[\s\S]*?background:\s*var\(--surface-raised\)/) | ||
| expect(select).not.toMatch(/&:hover\s*\{[\s\S]*?background:\s*var\(--surface-raised\)/) | ||
| }) | ||
| }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.