diff --git a/.claude/skills/simplify-code/SKILL.md b/.claude/skills/simplify-code/SKILL.md new file mode 100644 index 000000000..a5a4c16c6 --- /dev/null +++ b/.claude/skills/simplify-code/SKILL.md @@ -0,0 +1,299 @@ +--- +name: simplify-code +description: Simplifies code by minimizing complexity, eliminating duplication, and prioritizing legibility for contributors. Use when implementing features, fixing bugs, refactoring, or when the user asks to simplify, clean up, make DRY, reduce complexity, or improve maintainability. +--- + +# Simplify Code + +Favor minimal, legible implementations. Fewer lines and clearer structure make code easier to understand and maintain. + +## Before Making Changes + +1. **Necessity**: Does this change only address what's required? +2. **Existing abstractions**: Are there shared utilities, hooks, or helpers in `common/`, `util/`, etc.? +3. **Duplication**: Where else does similar logic exist? +4. **Root cause**: What is actually driving the complexity? + +## Core Principles + +### Minimal Surface Area + +- Prefer the smallest change that achieves the goal +- Add abstractions only when reuse is real and obvious +- Avoid "future-proofing" that adds complexity now + +### DRY + +- **Literal duplication**: Same logic in 2+ places → extract shared function +- **Similar structure**: Parameterize or compose instead of copying +- **Config-driven branches**: Use maps/objects instead of long switch/if chains +- **Do not over-DRY**: Don't unify logic that merely _looks_ similar; shared abstractions must cleanly handle all real cases + +### Legibility + +- One clear responsibility per function/component +- Guard clauses and early returns over deep nesting +- Functions under ~20 lines when feasible; flag functions over ~30 lines +- Each line readable without jumping around + +### Durability + +- Prefer built-ins and stable, minimal dependencies +- Abstractions with narrow, stable contracts +- Avoid hidden state and surprising side effects + +## When to Extract vs. Inline + +**Extract when:** + +- Logic is used in 2+ places with shared intent +- A block has a clear, reusable name +- The abstraction has a narrow, stable API + +**Do not extract when:** + +- Used once and clear inline +- The abstraction would need many params or special cases +- The abstraction name would be vague (e.g. `doStuff`) + +## Complexity Thresholds + +| Metric | Prefer | Flag | Action | +| -------------------- | ---------- | ---------- | ---------------------------- | +| Function length | < 20 lines | > 30 lines | Split or extract | +| Nesting depth | ≤ 2 levels | > 3 levels | Guard clauses, early returns | +| Parameters | ≤ 3 | > 4 | Options object or context | +| Conditional branches | ≤ 3 | > 4 | Map/object or polymorphism | +| Similar blocks | 0 | 2+ | Extract and parameterize | + +## DRY Detection Rules + +### Extract when you see + +- Same expression or block in 2+ places +- Copy-paste with variable name changes only +- Parallel structures (e.g. handlers for A and B that mirror each other) +- Repeated validation, formatting, or mapping logic + +### Parameterize when + +- Logic is identical except for a value or small behavior +- A good abstraction name exists (e.g. `formatDate`, `validateEmail`) +- The parameter surface is small (< 4 params typically) + +### Do not extract when + +- Used once and inline logic is clear +- Abstraction would need many optional params or flags +- Name would be generic (`handleThing`, `processData`) +- Only superficial similarity — real behavior diverges +- Combining would obscure intent or create hidden coupling + +## Nesting Reduction + +**Preferred**: Guard clauses and early returns + +``` +if (!user?.isActive) return; +if (!user.hasPermission) return; +doThing(); +``` + +**Avoid**: Deep nesting + +``` +if (user) { + if (user.isActive) { + if (user.hasPermission) { + doThing(); + } + } +} +``` + +## Config Over Conditionals + +**When**: Multiple similar branches based on a key or type + +**Instead of**: Long switch or if-else chain + +``` +switch (type) { + case 'a': return handleA(); + case 'b': return handleB(); + case 'c': return handleC(); +} +``` + +**Prefer**: Map or object lookup (when handlers are simple) + +``` +const handlers = { a: handleA, b: handleB, c: handleC }; +return handlers[type]?.(); +``` + +## Project-Aware Simplification + +Before adding new abstractions: + +1. Search for existing utilities in `common/`, `util/`, `hooks/` +2. Check sibling components or similar views for shared patterns +3. Follow established conventions (e.g. naming, file layout) +4. Prefer composition over new inheritance + +## Durability Checklist + +- [ ] Abstraction has a clear, narrow contract +- [ ] No hidden globals or mutable shared state +- [ ] Types/interfaces document inputs and outputs +- [ ] Dependencies are minimal and stable +- [ ] Change is incremental; no big-bang refactors + +## Anti-Patterns to Avoid + +- Abstracting "for the future" without current reuse +- Clever one-liners that obscure intent +- Premature micro-optimization over clarity +- Collapsing unrelated responsibilities into one abstraction +- Overly deep inheritance or generic hierarchies + +## Output Format + +When proposing simplifications: + +1. One-sentence summary of the change +2. Minimal diff or before/after +3. Principle(s) applied (DRY, legibility, etc.) +4. Any tradeoffs noted + +## Examples + +### Guard Clauses vs. Nesting + +**Before:** + +```ts +function processUser(user: User | null) { + if (user) { + if (user.isActive) { + if (user.hasPermission) { + return doThing(user); + } + } + } + return null; +} +``` + +**After:** + +```ts +function processUser(user: User | null) { + if (!user?.isActive || !user.hasPermission) return null; + return doThing(user); +} +``` + +### Single Pass vs. Repeated Iteration + +**Before:** + +```ts +const names = items.map((i) => i.name); +const ids = items.map((i) => i.id); +const active = items.filter((i) => i.active); +``` + +**After** (when two+ iterations over same array): + +```ts +const { names, ids, active } = items.reduce( + (acc, i) => ({ + names: [...acc.names, i.name], + ids: [...acc.ids, i.id], + active: i.active ? [...acc.active, i] : acc.active, + }), + { names: [] as string[], ids: [] as string[], active: [] as Item[] }, +); +``` + +_Note:_ Keep separate passes if they are clearer; avoid reduce when simple map/filter is more readable. + +### Config-Driven Handlers + +**Before:** + +```ts +function getLabel(type: string) { + if (type === "email") return "Email"; + if (type === "phone") return "Phone"; + if (type === "address") return "Address"; + return "Unknown"; +} +``` + +**After:** + +```ts +const LABELS: Record = { + email: "Email", + phone: "Phone", + address: "Address", +}; +const getLabel = (type: string) => LABELS[type] ?? "Unknown"; +``` + +### Duplicated Logic → Shared Helper + +**Before:** + +```ts +// In ComponentA +const formatted = `${user.firstName} ${user.lastName}`.trim(); + +// In ComponentB +const displayName = `${user.firstName} ${user.lastName}`.trim(); +``` + +**After:** + +```ts +// common/util/formatUser.ts +export const formatFullName = (user: { firstName: string; lastName: string }) => + `${user.firstName} ${user.lastName}`.trim(); +``` + +### Inline When Single Use + +**Before** (over-extraction): + +```ts +const getIsValid = (x: number) => x > 0 && x < 100; +if (getIsValid(value)) { ... } +``` + +**After:** + +```ts +if (value > 0 && value < 100) { ... } +``` + +### Composing Hooks Instead of Duplication + +**Before** (similar logic in two components): + +```ts +// DayCmdPalette.tsx +const authItems = isLoggedIn ? [logoutItem] : [loginItem, signupItem]; + +// NowCmdPalette.tsx +const authItems = isLoggedIn ? [logoutItem] : [loginItem, signupItem]; +``` + +**After:** + +```ts +// useAuthCmdItems.ts +export const useAuthCmdItems = (isLoggedIn: boolean) => + isLoggedIn ? [logoutItem] : [loginItem, signupItem]; +``` diff --git a/.codex/skills/simplify-code/SKILL.md b/.codex/skills/simplify-code/SKILL.md new file mode 100644 index 000000000..1d58dcdf0 --- /dev/null +++ b/.codex/skills/simplify-code/SKILL.md @@ -0,0 +1,79 @@ +--- +name: codex-simplify-code +description: Simplifies code by minimizing complexity, eliminating duplication, and prioritizing legibility for contributors. Use when implementing features, fixing bugs, refactoring, or when the user asks to simplify, clean up, make DRY, reduce complexity, or improve maintainability. +--- + +# Simplify Code + +Favor minimal, legible implementations. Fewer lines and clearer structure make code easier to understand and maintain. + +## Before Making Changes + +1. **Necessity**: Does this change only address what's required? +2. **Existing abstractions**: Are there shared utilities, hooks, or helpers in `common/`, `util/`, etc.? +3. **Duplication**: Where else does similar logic exist? +4. **Root cause**: What is actually driving the complexity? + +## Core Principles + +### Minimal Surface Area + +- Prefer the smallest change that achieves the goal +- Add abstractions only when reuse is real and obvious +- Avoid "future-proofing" that adds complexity now + +### DRY + +- **Literal duplication**: Same logic in 2+ places → extract shared function +- **Similar structure**: Parameterize or compose instead of copying +- **Config-driven branches**: Use maps/objects instead of long switch/if chains +- **Do not over-DRY**: Don't unify logic that merely _looks_ similar; shared abstractions must cleanly handle all real cases + +### Legibility + +- One clear responsibility per function/component +- Guard clauses and early returns over deep nesting +- Functions under ~20 lines when feasible; flag functions over ~30 lines +- Each line readable without jumping around + +### Durability + +- Prefer built-ins and stable, minimal dependencies +- Abstractions with narrow, stable contracts +- Avoid hidden state and surprising side effects + +## When to Extract vs. Inline + +**Extract when:** + +- Logic is used in 2+ places with shared intent +- A block has a clear, reusable name +- The abstraction has a narrow, stable API + +**Do not extract when:** + +- Used once and clear inline +- The abstraction would need many params or special cases +- The abstraction name would be vague (e.g. `doStuff`) + +## Anti-Patterns to Avoid + +- Abstracting "for the future" without current reuse +- Clever one-liners that obscure intent +- Premature micro-optimization over clarity +- Collapsing unrelated responsibilities into one abstraction +- Overly deep inheritance or generic hierarchies + +## Output Format + +When proposing simplifications: + +1. One-sentence summary of the change +2. Minimal diff or before/after +3. Principle(s) applied (DRY, legibility, etc.) +4. Any tradeoffs noted + +## Additional Resources + +- For detailed heuristics and decision rules, see [heuristics.md](heuristics.md) +- For before/after patterns, see [examples.md](examples.md) diff --git a/.codex/skills/simplify-code/examples.md b/.codex/skills/simplify-code/examples.md new file mode 100644 index 000000000..fb4a02e32 --- /dev/null +++ b/.codex/skills/simplify-code/examples.md @@ -0,0 +1,131 @@ +# Simplify Code — Examples + +## Guard Clauses vs. Nesting + +**Before:** + +```ts +function processUser(user: User | null) { + if (user) { + if (user.isActive) { + if (user.hasPermission) { + return doThing(user); + } + } + } + return null; +} +``` + +**After:** + +```ts +function processUser(user: User | null) { + if (!user?.isActive || !user.hasPermission) return null; + return doThing(user); +} +``` + +## Single Pass vs. Repeated Iteration + +**Before:** + +```ts +const names = items.map((i) => i.name); +const ids = items.map((i) => i.id); +const active = items.filter((i) => i.active); +``` + +**After** (when two+ iterations over same array): + +```ts +const { names, ids, active } = items.reduce( + (acc, i) => ({ + names: [...acc.names, i.name], + ids: [...acc.ids, i.id], + active: i.active ? [...acc.active, i] : acc.active, + }), + { names: [] as string[], ids: [] as string[], active: [] as Item[] }, +); +``` + +_Note:_ Keep separate passes if they are clearer; avoid reduce when simple map/filter is more readable. + +## Config-Driven Handlers + +**Before:** + +```ts +function getLabel(type: string) { + if (type === "email") return "Email"; + if (type === "phone") return "Phone"; + if (type === "address") return "Address"; + return "Unknown"; +} +``` + +**After:** + +```ts +const LABELS: Record = { + email: "Email", + phone: "Phone", + address: "Address", +}; +const getLabel = (type: string) => LABELS[type] ?? "Unknown"; +``` + +## Duplicated Logic → Shared Helper + +**Before:** + +```ts +// In ComponentA +const formatted = `${user.firstName} ${user.lastName}`.trim(); + +// In ComponentB +const displayName = `${user.firstName} ${user.lastName}`.trim(); +``` + +**After:** + +```ts +// common/util/formatUser.ts +export const formatFullName = (user: { firstName: string; lastName: string }) => + `${user.firstName} ${user.lastName}`.trim(); +``` + +## Inline When Single Use + +**Before** (over-extraction): + +```ts +const getIsValid = (x: number) => x > 0 && x < 100; +if (getIsValid(value)) { ... } +``` + +**After:** + +```ts +if (value > 0 && value < 100) { ... } +``` + +## Composing Hooks Instead of Duplication + +**Before** (similar logic in two components): + +```ts +// DayCmdPalette.tsx +const authItems = isLoggedIn ? [logoutItem] : [loginItem, signupItem]; + +// NowCmdPalette.tsx +const authItems = isLoggedIn ? [logoutItem] : [loginItem, signupItem]; +``` + +**After:** + +```ts +// useAuthCmdItems.ts +export const useAuthCmdItems = (isLoggedIn: boolean) => + isLoggedIn ? [logoutItem] : [loginItem, signupItem]; +``` diff --git a/.codex/skills/simplify-code/heuristics.md b/.codex/skills/simplify-code/heuristics.md new file mode 100644 index 000000000..50d68ed50 --- /dev/null +++ b/.codex/skills/simplify-code/heuristics.md @@ -0,0 +1,94 @@ +# Simplify Code — Heuristics + +## Complexity Thresholds + +| Metric | Prefer | Flag | Action | +| -------------------- | ---------- | ---------- | ---------------------------- | +| Function length | < 20 lines | > 30 lines | Split or extract | +| Nesting depth | ≤ 2 levels | > 3 levels | Guard clauses, early returns | +| Parameters | ≤ 3 | > 4 | Options object or context | +| Conditional branches | ≤ 3 | > 4 | Map/object or polymorphism | +| Similar blocks | 0 | 2+ | Extract and parameterize | + +## DRY Detection Rules + +### Extract when you see + +- Same expression or block in 2+ places +- Copy-paste with variable name changes only +- Parallel structures (e.g. handlers for A and B that mirror each other) +- Repeated validation, formatting, or mapping logic + +### Parameterize when + +- Logic is identical except for a value or small behavior +- A good abstraction name exists (e.g. `formatDate`, `validateEmail`) +- The parameter surface is small (< 4 params typically) + +### Do not extract when + +- Used once and inline logic is clear +- Abstraction would need many optional params or flags +- Name would be generic (`handleThing`, `processData`) +- Only superficial similarity — real behavior diverges +- Combining would obscure intent or create hidden coupling + +## Nesting Reduction + +**Preferred**: Guard clauses and early returns + +``` +if (!user?.isActive) return; +if (!user.hasPermission) return; +doThing(); +``` + +**Avoid**: Deep nesting + +``` +if (user) { + if (user.isActive) { + if (user.hasPermission) { + doThing(); + } + } +} +``` + +## Config Over Conditionals + +**When**: Multiple similar branches based on a key or type + +**Instead of**: Long switch or if-else chain + +``` +switch (type) { + case 'a': return handleA(); + case 'b': return handleB(); + case 'c': return handleC(); +} +``` + +**Prefer**: Map or object lookup (when handlers are simple) + +``` +const handlers = { a: handleA, b: handleB, c: handleC }; +return handlers[type]?.(); +``` + +## Project-Aware Simplification + +Before adding new abstractions: + +1. Search for existing utilities in `common/`, `util/`, `hooks/` +2. Check sibling components or similar views for shared patterns +3. Follow established conventions (e.g. naming, file layout) +4. Prefer composition over new inheritance + +## Durability Checklist + +- [ ] Abstraction has a clear, narrow contract +- [ ] No hidden globals or mutable shared state +- [ ] Types/interfaces document inputs and outputs +- [ ] Dependencies are minimal and stable +- [ ] Change is incremental; no big-bang refactors diff --git a/.cursor/rules/naming-convention.mdc b/.cursor/rules/naming-convention.mdc new file mode 100644 index 000000000..3dca9096c --- /dev/null +++ b/.cursor/rules/naming-convention.mdc @@ -0,0 +1,3 @@ +--- +alwaysApply: true +--- diff --git a/.cursor/skills/simplify-code/SKILL.md b/.cursor/skills/simplify-code/SKILL.md new file mode 100644 index 000000000..563036126 --- /dev/null +++ b/.cursor/skills/simplify-code/SKILL.md @@ -0,0 +1,79 @@ +--- +name: simplify-code +description: Simplifies code by minimizing complexity, eliminating duplication, and prioritizing legibility for contributors. Use when implementing features, fixing bugs, refactoring, or when the user asks to simplify, clean up, make DRY, reduce complexity, or improve maintainability. +--- + +# Simplify Code + +Favor minimal, legible implementations. Fewer lines and clearer structure make code easier to understand and maintain. + +## Before Making Changes + +1. **Necessity**: Does this change only address what's required? +2. **Existing abstractions**: Are there shared utilities, hooks, or helpers in `common/`, `util/`, etc.? +3. **Duplication**: Where else does similar logic exist? +4. **Root cause**: What is actually driving the complexity? + +## Core Principles + +### Minimal Surface Area + +- Prefer the smallest change that achieves the goal +- Add abstractions only when reuse is real and obvious +- Avoid "future-proofing" that adds complexity now + +### DRY + +- **Literal duplication**: Same logic in 2+ places → extract shared function +- **Similar structure**: Parameterize or compose instead of copying +- **Config-driven branches**: Use maps/objects instead of long switch/if chains +- **Do not over-DRY**: Don't unify logic that merely _looks_ similar; shared abstractions must cleanly handle all real cases + +### Legibility + +- One clear responsibility per function/component +- Guard clauses and early returns over deep nesting +- Functions under ~20 lines when feasible; flag functions over ~30 lines +- Each line readable without jumping around + +### Durability + +- Prefer built-ins and stable, minimal dependencies +- Abstractions with narrow, stable contracts +- Avoid hidden state and surprising side effects + +## When to Extract vs. Inline + +**Extract when:** + +- Logic is used in 2+ places with shared intent +- A block has a clear, reusable name +- The abstraction has a narrow, stable API + +**Do not extract when:** + +- Used once and clear inline +- The abstraction would need many params or special cases +- The abstraction name would be vague (e.g. `doStuff`) + +## Anti-Patterns to Avoid + +- Abstracting "for the future" without current reuse +- Clever one-liners that obscure intent +- Premature micro-optimization over clarity +- Collapsing unrelated responsibilities into one abstraction +- Overly deep inheritance or generic hierarchies + +## Output Format + +When proposing simplifications: + +1. One-sentence summary of the change +2. Minimal diff or before/after +3. Principle(s) applied (DRY, legibility, etc.) +4. Any tradeoffs noted + +## Additional Resources + +- For detailed heuristics and decision rules, see [heuristics.md](heuristics.md) +- For before/after patterns, see [examples.md](examples.md) diff --git a/.cursor/skills/simplify-code/examples.md b/.cursor/skills/simplify-code/examples.md new file mode 100644 index 000000000..fb4a02e32 --- /dev/null +++ b/.cursor/skills/simplify-code/examples.md @@ -0,0 +1,131 @@ +# Simplify Code — Examples + +## Guard Clauses vs. Nesting + +**Before:** + +```ts +function processUser(user: User | null) { + if (user) { + if (user.isActive) { + if (user.hasPermission) { + return doThing(user); + } + } + } + return null; +} +``` + +**After:** + +```ts +function processUser(user: User | null) { + if (!user?.isActive || !user.hasPermission) return null; + return doThing(user); +} +``` + +## Single Pass vs. Repeated Iteration + +**Before:** + +```ts +const names = items.map((i) => i.name); +const ids = items.map((i) => i.id); +const active = items.filter((i) => i.active); +``` + +**After** (when two+ iterations over same array): + +```ts +const { names, ids, active } = items.reduce( + (acc, i) => ({ + names: [...acc.names, i.name], + ids: [...acc.ids, i.id], + active: i.active ? [...acc.active, i] : acc.active, + }), + { names: [] as string[], ids: [] as string[], active: [] as Item[] }, +); +``` + +_Note:_ Keep separate passes if they are clearer; avoid reduce when simple map/filter is more readable. + +## Config-Driven Handlers + +**Before:** + +```ts +function getLabel(type: string) { + if (type === "email") return "Email"; + if (type === "phone") return "Phone"; + if (type === "address") return "Address"; + return "Unknown"; +} +``` + +**After:** + +```ts +const LABELS: Record = { + email: "Email", + phone: "Phone", + address: "Address", +}; +const getLabel = (type: string) => LABELS[type] ?? "Unknown"; +``` + +## Duplicated Logic → Shared Helper + +**Before:** + +```ts +// In ComponentA +const formatted = `${user.firstName} ${user.lastName}`.trim(); + +// In ComponentB +const displayName = `${user.firstName} ${user.lastName}`.trim(); +``` + +**After:** + +```ts +// common/util/formatUser.ts +export const formatFullName = (user: { firstName: string; lastName: string }) => + `${user.firstName} ${user.lastName}`.trim(); +``` + +## Inline When Single Use + +**Before** (over-extraction): + +```ts +const getIsValid = (x: number) => x > 0 && x < 100; +if (getIsValid(value)) { ... } +``` + +**After:** + +```ts +if (value > 0 && value < 100) { ... } +``` + +## Composing Hooks Instead of Duplication + +**Before** (similar logic in two components): + +```ts +// DayCmdPalette.tsx +const authItems = isLoggedIn ? [logoutItem] : [loginItem, signupItem]; + +// NowCmdPalette.tsx +const authItems = isLoggedIn ? [logoutItem] : [loginItem, signupItem]; +``` + +**After:** + +```ts +// useAuthCmdItems.ts +export const useAuthCmdItems = (isLoggedIn: boolean) => + isLoggedIn ? [logoutItem] : [loginItem, signupItem]; +``` diff --git a/.cursor/skills/simplify-code/heuristics.md b/.cursor/skills/simplify-code/heuristics.md new file mode 100644 index 000000000..50d68ed50 --- /dev/null +++ b/.cursor/skills/simplify-code/heuristics.md @@ -0,0 +1,94 @@ +# Simplify Code — Heuristics + +## Complexity Thresholds + +| Metric | Prefer | Flag | Action | +| -------------------- | ---------- | ---------- | ---------------------------- | +| Function length | < 20 lines | > 30 lines | Split or extract | +| Nesting depth | ≤ 2 levels | > 3 levels | Guard clauses, early returns | +| Parameters | ≤ 3 | > 4 | Options object or context | +| Conditional branches | ≤ 3 | > 4 | Map/object or polymorphism | +| Similar blocks | 0 | 2+ | Extract and parameterize | + +## DRY Detection Rules + +### Extract when you see + +- Same expression or block in 2+ places +- Copy-paste with variable name changes only +- Parallel structures (e.g. handlers for A and B that mirror each other) +- Repeated validation, formatting, or mapping logic + +### Parameterize when + +- Logic is identical except for a value or small behavior +- A good abstraction name exists (e.g. `formatDate`, `validateEmail`) +- The parameter surface is small (< 4 params typically) + +### Do not extract when + +- Used once and inline logic is clear +- Abstraction would need many optional params or flags +- Name would be generic (`handleThing`, `processData`) +- Only superficial similarity — real behavior diverges +- Combining would obscure intent or create hidden coupling + +## Nesting Reduction + +**Preferred**: Guard clauses and early returns + +``` +if (!user?.isActive) return; +if (!user.hasPermission) return; +doThing(); +``` + +**Avoid**: Deep nesting + +``` +if (user) { + if (user.isActive) { + if (user.hasPermission) { + doThing(); + } + } +} +``` + +## Config Over Conditionals + +**When**: Multiple similar branches based on a key or type + +**Instead of**: Long switch or if-else chain + +``` +switch (type) { + case 'a': return handleA(); + case 'b': return handleB(); + case 'c': return handleC(); +} +``` + +**Prefer**: Map or object lookup (when handlers are simple) + +``` +const handlers = { a: handleA, b: handleB, c: handleC }; +return handlers[type]?.(); +``` + +## Project-Aware Simplification + +Before adding new abstractions: + +1. Search for existing utilities in `common/`, `util/`, `hooks/` +2. Check sibling components or similar views for shared patterns +3. Follow established conventions (e.g. naming, file layout) +4. Prefer composition over new inheritance + +## Durability Checklist + +- [ ] Abstraction has a clear, narrow contract +- [ ] No hidden globals or mutable shared state +- [ ] Types/interfaces document inputs and outputs +- [ ] Dependencies are minimal and stable +- [ ] Change is incremental; no big-bang refactors diff --git a/.gitignore b/.gitignore index 666567fd6..6b369e476 100644 --- a/.gitignore +++ b/.gitignore @@ -20,7 +20,6 @@ packages/**/yarn.lock ######## # root .claude/settings.local.json -.cursor/ .idea/ .mcp.json .vscode/ diff --git a/e2e/utils/event-test-utils.ts b/e2e/utils/event-test-utils.ts index ad79b938a..467489180 100644 --- a/e2e/utils/event-test-utils.ts +++ b/e2e/utils/event-test-utils.ts @@ -213,6 +213,8 @@ export const fillTitleAndSaveWithKeyboard = async ( await expect(titleInput).toBeVisible({ timeout: FORM_TIMEOUT }); await titleInput.fill(title); await page.keyboard.press("Enter"); + // Wait for form to close, confirming the save completed + await titleInput.waitFor({ state: "hidden", timeout: FORM_TIMEOUT }); }; export const openTimedEventFormWithMouse = async (page: Page) => { diff --git a/packages/web/src/common/hooks/useAuthCmdItems.test.ts b/packages/web/src/common/hooks/useAuthCmdItems.test.ts new file mode 100644 index 000000000..5d755c8a9 --- /dev/null +++ b/packages/web/src/common/hooks/useAuthCmdItems.test.ts @@ -0,0 +1,85 @@ +import type { MouseEvent } from "react"; +import { act } from "react"; +import { renderHook } from "@testing-library/react"; +import { useSession } from "@web/auth/hooks/session/useSession"; +import { useAuthModal } from "@web/components/AuthModal/hooks/useAuthModal"; +import { useAuthCmdItems } from "./useAuthCmdItems"; + +jest.mock("@web/auth/hooks/session/useSession", () => ({ + useSession: jest.fn(), +})); + +jest.mock("@web/components/AuthModal/hooks/useAuthModal", () => ({ + useAuthModal: jest.fn(), +})); + +describe("useAuthCmdItems", () => { + const mockOpenModal = jest.fn(); + const mockUseSession = useSession as jest.MockedFunction; + const mockUseAuthModal = useAuthModal as jest.MockedFunction< + typeof useAuthModal + >; + + beforeEach(() => { + jest.clearAllMocks(); + window.history.pushState({}, "", "/day"); + + mockUseSession.mockReturnValue({ + authenticated: false, + setAuthenticated: jest.fn(), + }); + mockUseAuthModal.mockReturnValue({ + isOpen: false, + currentView: "login", + openModal: mockOpenModal, + closeModal: jest.fn(), + setView: jest.fn(), + }); + }); + + it("returns no items when authenticated", () => { + window.history.pushState({}, "", "/day?auth=true"); + mockUseSession.mockReturnValue({ + authenticated: true, + setAuthenticated: jest.fn(), + }); + + const { result } = renderHook(() => useAuthCmdItems()); + + expect(result.current).toEqual([]); + }); + + it("returns no items when auth feature flag is disabled", () => { + const { result } = renderHook(() => useAuthCmdItems()); + + expect(result.current).toEqual([]); + }); + + it("returns auth items when unauthenticated and auth feature flag is enabled", () => { + window.history.pushState({}, "", "/day?auth=true"); + + const { result } = renderHook(() => useAuthCmdItems()); + + expect(result.current.map((item) => item.id)).toEqual([ + "sign-up", + "log-in", + ]); + }); + + it("opens matching auth modal view when item actions are clicked", () => { + window.history.pushState({}, "", "/day?auth=true"); + + const { result } = renderHook(() => useAuthCmdItems()); + const signUpItem = result.current.find((item) => item.id === "sign-up"); + const logInItem = result.current.find((item) => item.id === "log-in"); + + const mockEvent = {} as MouseEvent; + act(() => { + signUpItem?.onClick?.(mockEvent); + logInItem?.onClick?.(mockEvent); + }); + + expect(mockOpenModal).toHaveBeenNthCalledWith(1, "signUp"); + expect(mockOpenModal).toHaveBeenNthCalledWith(2, "login"); + }); +}); diff --git a/packages/web/src/common/hooks/useAuthCmdItems.ts b/packages/web/src/common/hooks/useAuthCmdItems.ts new file mode 100644 index 000000000..451cb2ca1 --- /dev/null +++ b/packages/web/src/common/hooks/useAuthCmdItems.ts @@ -0,0 +1,34 @@ +import { JsonStructureItem } from "react-cmdk"; +import { useSession } from "@web/auth/hooks/session/useSession"; +import { useAuthModal } from "@web/components/AuthModal/hooks/useAuthModal"; + +/** + * Returns command palette items for authentication actions. + * Items are only returned when the user is not authenticated. + */ +export const useAuthCmdItems = (): JsonStructureItem[] => { + const { authenticated } = useSession(); + const { openModal } = useAuthModal(); + const isAuthFeatureEnabled = + typeof window !== "undefined" && + new URLSearchParams(window.location.search).has("auth"); + + if (authenticated || !isAuthFeatureEnabled) { + return []; + } + + return [ + { + id: "sign-up", + children: "Sign Up", + icon: "UserPlusIcon", + onClick: () => openModal("signUp"), + }, + { + id: "log-in", + children: "Log In", + icon: "ArrowLeftOnRectangleIcon", + onClick: () => openModal("login"), + }, + ]; +}; diff --git a/packages/web/src/components/AuthModal/components/AuthButton.tsx b/packages/web/src/components/AuthModal/components/AuthButton.tsx index 5e0cb29d5..c6d26f91d 100644 --- a/packages/web/src/components/AuthModal/components/AuthButton.tsx +++ b/packages/web/src/components/AuthModal/components/AuthButton.tsx @@ -31,7 +31,7 @@ export const AuthButton: FC = ({ - -
- -

- Browser Storage -

-
- -
-

- Day tasks are saved in your browser's local storage. Clearing - your browser data will remove them. -

-

Think of day tasks as simple ways to stay focused on today.

-

- Your calendar events are safely backed up and not - stored in your browser. -

-
- -
- -
- - - ); -}; diff --git a/packages/web/src/views/Now/components/NowCmdPalette.tsx b/packages/web/src/views/Now/components/NowCmdPalette.tsx index 28cae9df0..06b4e6238 100644 --- a/packages/web/src/views/Now/components/NowCmdPalette.tsx +++ b/packages/web/src/views/Now/components/NowCmdPalette.tsx @@ -4,6 +4,7 @@ import "react-cmdk/dist/cmdk.css"; import { useConnectGoogle } from "@web/auth/hooks/oauth/useConnectGoogle"; import { moreCommandPaletteItems } from "@web/common/constants/more.cmd.constants"; import { VIEW_SHORTCUTS } from "@web/common/constants/shortcuts.constants"; +import { useAuthCmdItems } from "@web/common/hooks/useAuthCmdItems"; import { pressKey } from "@web/common/utils/dom/event-emitter.util"; import { onEventTargetVisibility } from "@web/common/utils/dom/event-target-visibility.util"; import { selectIsCmdPaletteOpen } from "@web/ducks/settings/selectors/settings.selectors"; @@ -17,6 +18,7 @@ export const NowCmdPalette = () => { const [search, setSearch] = useState(""); const { isGoogleCalendarConnected, onConnectGoogleCalendar } = useConnectGoogle(); + const authCmdItems = useAuthCmdItems(); const filteredItems = filterItems( [ @@ -60,6 +62,7 @@ export const NowCmdPalette = () => { ? undefined : onConnectGoogleCalendar, }, + ...authCmdItems, { id: "log-out", children: "Log Out [z]",