Skip to content

Commit 218da21

Browse files
fix: emoji picker and page icon design spec violations (#186)
- Emoji buttons: add 44px minimum touch targets on mobile (sm:min-h-8 on desktop) - Page icon button: add 44px minimum touch targets on mobile - Filter input: add border border-white/[0.06] and focus:ring-2 focus:ring-ring - Page icon "Add icon" button: show on mobile with max-sm:opacity-100 - Emoji picker: use viewport-relative width on mobile to prevent clipping - Add static analysis regression tests for all four violations Co-authored-by: Ona <no-reply@ona.com>
1 parent 5df0ce5 commit 218da21

3 files changed

Lines changed: 88 additions & 6 deletions

File tree

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import { describe, it, expect } from "vitest";
2+
import { readFileSync } from "fs";
3+
import { resolve } from "path";
4+
5+
/**
6+
* Regression tests for issue #186: UI does not match design spec after PR #185.
7+
*
8+
* These tests verify that emoji-picker and page-icon source files comply with
9+
* the design spec constraints for touch targets, input styling, mobile
10+
* visibility, and responsive width.
11+
*/
12+
13+
function readSource(relativePath: string): string {
14+
return readFileSync(resolve(__dirname, relativePath), "utf-8");
15+
}
16+
17+
describe("emoji-picker design spec compliance", () => {
18+
const source = readSource("./emoji-picker.tsx");
19+
20+
it("emoji buttons have 44px minimum touch target on mobile", () => {
21+
// Design spec: "Touch targets: minimum 44px on mobile."
22+
// Emoji buttons must use min-h-[44px] and min-w-[44px] for mobile.
23+
expect(source).toContain("min-h-[44px]");
24+
expect(source).toContain("min-w-[44px]");
25+
});
26+
27+
it("filter input has border per input spec", () => {
28+
// Design spec: Inputs should have border-white/[0.06] border.
29+
const precedingClass = source.match(
30+
/className="[^"]*border border-white\/\[0\.06\][^"]*"[\s\S]*?aria-label="Filter emojis"/
31+
);
32+
expect(precedingClass).not.toBeNull();
33+
});
34+
35+
it("filter input has focus ring per input spec", () => {
36+
// Design spec: Inputs should have ring-2 ring-ring on focus.
37+
const inputSection = source.match(
38+
/className="[^"]*"[\s\S]*?aria-label="Filter emojis"/
39+
);
40+
expect(inputSection).not.toBeNull();
41+
expect(inputSection![0]).toContain("focus:ring-2");
42+
expect(inputSection![0]).toContain("focus:ring-ring");
43+
});
44+
45+
it("picker uses responsive width to avoid mobile clipping", () => {
46+
// Design spec: "No horizontal scroll on any breakpoint."
47+
// The picker must not use a fixed w-72 without a mobile-responsive alternative.
48+
expect(source).toContain("sm:w-72");
49+
// Must have a viewport-relative width for mobile
50+
expect(source).toMatch(/w-\[calc\(100vw/);
51+
});
52+
});
53+
54+
describe("page-icon design spec compliance", () => {
55+
const source = readSource("./page-icon.tsx");
56+
57+
it("page icon button has 44px minimum touch target on mobile", () => {
58+
// Design spec: "Touch targets: minimum 44px on mobile."
59+
expect(source).toContain("min-h-[44px]");
60+
expect(source).toContain("min-w-[44px]");
61+
});
62+
63+
it("add-icon button is visible on mobile (not hover-only)", () => {
64+
// Design spec: Touch targets must be accessible on mobile.
65+
// The button container must not rely solely on group-hover for visibility.
66+
// It should use max-sm:opacity-100 or similar to be visible on touch devices.
67+
expect(source).toContain("max-sm:opacity-100");
68+
});
69+
70+
it("add-icon button has 44px minimum touch target on mobile", () => {
71+
// The "Add icon" button must meet the 44px minimum on mobile.
72+
const addIconSection = source.match(
73+
/aria-label="Add page icon"[\s\S]{0,50}/
74+
);
75+
expect(addIconSection).not.toBeNull();
76+
77+
const classSection = source.match(
78+
/className="[^"]*min-h-\[44px\][^"]*"[\s\S]*?aria-label="Add page icon"/
79+
);
80+
expect(classSection).not.toBeNull();
81+
});
82+
});

src/components/emoji-picker.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ export function EmojiPicker({
181181
{open && (
182182
<div
183183
ref={floatingRef}
184-
className="fixed z-50 w-72 rounded-sm border border-white/[0.06] bg-popover p-2 shadow-md"
184+
className="fixed z-50 w-[calc(100vw-16px)] rounded-sm border border-white/[0.06] bg-popover p-2 shadow-md sm:w-72"
185185
role="dialog"
186186
aria-label="Emoji picker"
187187
>
@@ -191,7 +191,7 @@ export function EmojiPicker({
191191
value={filter}
192192
onChange={(e) => setFilter(e.target.value)}
193193
placeholder="Filter..."
194-
className="mb-2 w-full bg-muted px-2 py-1 text-sm text-foreground placeholder:text-muted-foreground outline-none"
194+
className="mb-2 h-9 w-full border border-white/[0.06] bg-muted px-2 py-1 text-sm text-foreground placeholder:text-muted-foreground focus:ring-2 focus:ring-ring focus:outline-none"
195195
aria-label="Filter emojis"
196196
/>
197197
{hasIcon && onRemove && (
@@ -213,7 +213,7 @@ export function EmojiPicker({
213213
<button
214214
key={emoji}
215215
onClick={() => handleSelect(emoji)}
216-
className="flex h-8 w-8 items-center justify-center text-lg hover:bg-white/[0.04]"
216+
className="flex min-h-[44px] min-w-[44px] items-center justify-center text-lg hover:bg-white/[0.04] sm:min-h-8 sm:min-w-8"
217217
aria-label={`Select ${emoji}`}
218218
>
219219
{emoji}

src/components/page-icon.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export function PageIcon({ pageId, initialIcon }: PageIconProps) {
5555
hasIcon
5656
>
5757
<button
58-
className="flex h-10 w-10 items-center justify-center rounded-sm text-4xl leading-none hover:bg-white/[0.04]"
58+
className="flex min-h-[44px] min-w-[44px] items-center justify-center rounded-sm text-4xl leading-none hover:bg-white/[0.04] sm:min-h-10 sm:min-w-10"
5959
aria-label={`Page icon: ${icon}. Click to change`}
6060
>
6161
{icon}
@@ -66,7 +66,7 @@ export function PageIcon({ pageId, initialIcon }: PageIconProps) {
6666
}
6767

6868
return (
69-
<div className="mb-1 opacity-0 group-hover/page-header:opacity-100 transition-opacity">
69+
<div className="mb-1 max-sm:opacity-100 opacity-0 group-hover/page-header:opacity-100 transition-opacity">
7070
<EmojiPicker
7171
open={open}
7272
onOpenChange={setOpen}
@@ -75,7 +75,7 @@ export function PageIcon({ pageId, initialIcon }: PageIconProps) {
7575
hasIcon={false}
7676
>
7777
<button
78-
className="flex h-7 items-center gap-1 rounded-sm px-1.5 text-xs text-muted-foreground hover:bg-white/[0.04]"
78+
className="flex min-h-[44px] items-center gap-1 rounded-sm px-1.5 text-xs text-muted-foreground hover:bg-white/[0.04] sm:min-h-7"
7979
aria-label="Add page icon"
8080
>
8181
<SmilePlus className="h-3.5 w-3.5" />

0 commit comments

Comments
 (0)