Skip to content

Commit c3a7443

Browse files
fix: replace arbitrary spacing values with Tailwind scale equivalents (#201)
Co-authored-by: Ona <no-reply@ona.com>
1 parent ca098ff commit c3a7443

3 files changed

Lines changed: 23 additions & 14 deletions

File tree

src/components/emoji-picker-design-spec.test.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import { readFileSync } from "fs";
33
import { resolve } from "path";
44

55
/**
6-
* Regression tests for issue #186: UI does not match design spec after PR #185.
6+
* Regression tests for issues #186 and #201: UI design spec compliance.
77
*
88
* These tests verify that emoji-picker and page-icon source files comply with
99
* the design spec constraints for touch targets, input styling, mobile
10-
* visibility, and responsive width.
10+
* visibility, responsive width, and Tailwind spacing scale usage.
1111
*/
1212

1313
function readSource(relativePath: string): string {
@@ -17,11 +17,15 @@ function readSource(relativePath: string): string {
1717
describe("emoji-picker design spec compliance", () => {
1818
const source = readSource("./emoji-picker.tsx");
1919

20-
it("emoji buttons have 44px minimum touch target on mobile", () => {
20+
it("emoji buttons have 44px minimum touch target on mobile using Tailwind scale", () => {
2121
// 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]");
22+
// Design spec: "Never use arbitrary spacing values. Use the scale."
23+
// min-h-11 = 44px (11 × 4px), min-w-11 = 44px — exact Tailwind scale matches.
24+
expect(source).toContain("min-h-11");
25+
expect(source).toContain("min-w-11");
26+
// Must NOT use arbitrary values when a scale equivalent exists
27+
expect(source).not.toContain("min-h-[44px]");
28+
expect(source).not.toContain("min-w-[44px]");
2529
});
2630

2731
it("filter input has border per input spec", () => {
@@ -54,10 +58,14 @@ describe("emoji-picker design spec compliance", () => {
5458
describe("page-icon design spec compliance", () => {
5559
const source = readSource("./page-icon.tsx");
5660

57-
it("page icon button has 44px minimum touch target on mobile", () => {
61+
it("page icon button has 44px minimum touch target on mobile using Tailwind scale", () => {
5862
// Design spec: "Touch targets: minimum 44px on mobile."
59-
expect(source).toContain("min-h-[44px]");
60-
expect(source).toContain("min-w-[44px]");
63+
// Design spec: "Never use arbitrary spacing values. Use the scale."
64+
// min-h-11 = 44px, min-w-11 = 44px — exact Tailwind scale matches.
65+
expect(source).toContain("min-h-11");
66+
expect(source).toContain("min-w-11");
67+
expect(source).not.toContain("min-h-[44px]");
68+
expect(source).not.toContain("min-w-[44px]");
6169
});
6270

6371
it("add-icon button is visible on mobile (not hover-only)", () => {
@@ -67,15 +75,16 @@ describe("page-icon design spec compliance", () => {
6775
expect(source).toContain("max-sm:opacity-100");
6876
});
6977

70-
it("add-icon button has 44px minimum touch target on mobile", () => {
78+
it("add-icon button has 44px minimum touch target on mobile using Tailwind scale", () => {
7179
// The "Add icon" button must meet the 44px minimum on mobile.
80+
// Design spec: "Never use arbitrary spacing values. Use the scale."
7281
const addIconSection = source.match(
7382
/aria-label="Add page icon"[\s\S]{0,50}/
7483
);
7584
expect(addIconSection).not.toBeNull();
7685

7786
const classSection = source.match(
78-
/className="[^"]*min-h-\[44px\][^"]*"[\s\S]*?aria-label="Add page icon"/
87+
/className="[^"]*min-h-11[^"]*"[\s\S]*?aria-label="Add page icon"/
7988
);
8089
expect(classSection).not.toBeNull();
8190
});

src/components/emoji-picker.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ export function EmojiPicker({
213213
<button
214214
key={emoji}
215215
onClick={() => handleSelect(emoji)}
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"
216+
className="flex min-h-11 min-w-11 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: 2 additions & 2 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 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"
58+
className="flex min-h-11 min-w-11 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}
@@ -75,7 +75,7 @@ export function PageIcon({ pageId, initialIcon }: PageIconProps) {
7575
hasIcon={false}
7676
>
7777
<button
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"
78+
className="flex min-h-11 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)