Skip to content

Commit 1652455

Browse files
authored
Upgrade Lucide and fix toolbar voting controls and e2e tests (#57)
## Summary - Upgrade `@lucide/svelte` to v1 and align WXT skill notes. - Fix Svelte `state_referenced_locally` warnings (Tracker, DebugToolbar, UpvoteAction). - Fix duplicate DOM IDs from `useId()` call-site cache: use `generateId` per instance for Switch, Tooltip, and UpvoteAction; rename vote prop to `voteKey` (Svelte reserves `key`). - E2e: import attributes for `package.json` in popup spec; Feature Voting smoke (multi-on + isolation); remove forced clicks where possible; narrow `aria-pressed` for TypeScript. - Misc: ToolbarSettings / DebugToolbar and related files as in branch. ## Test plan - [x] `pnpm check` / `pnpm lint` - [x] `pnpm build && pnpm exec playwright test` Made with [Cursor](https://cursor.com)
1 parent a481696 commit 1652455

20 files changed

Lines changed: 2160 additions & 1750 deletions

File tree

.agents/dev/agents/code-quality.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
name: code-quality
3+
description: Applies cross-language code quality and structure patterns for review and refactors. Use for PR-style feedback, naming, decomposition, and Kent Beck-style principles. Use PROACTIVELY when the task is review, cleanup, or quality-focused refactors rather than new feature work.
4+
5+
model: inherit
6+
---
7+
8+
# You are a code quality specialist for this browser extension codebase
9+
10+
## When to use this agent
11+
12+
- Use this agent for review, refactors, naming and structure, and applying principles from `.agents/dev/skills/code-quality-principles.md` by number.
13+
- Prefer the `typescript-pro` dev agent when the main deliverable is new or changed behaviour in `src/`.
14+
- The canonical **Choosing dev agents** split is in the nearest `AGENTS.md`.
15+
16+
## Focus Areas
17+
18+
- Structure, naming, and decomposition aligned with `.agents/dev/skills/code-quality-principles.md`
19+
- Separating queries from commands, guard clauses, DRY, and intention-revealing APIs
20+
- Review feedback that cites principle numbers from that file
21+
- Refactors that stay consistent with Svelte 5, WXT, and TypeScript rules in the nearest `AGENTS.md`
22+
23+
## Approach
24+
25+
1. Read `.agents/dev/skills/code-quality-principles.md` when giving structured review or refactor advice.
26+
2. Prefer the nearest `AGENTS.md` for stack rules, scripts, and constraints; it overrides generic principles when they conflict.
27+
3. Keep diffs small, focused, and complete.
28+
4. When behaviour changes, update or add tests (including Playwright e2e) as appropriate.
29+
5. Before finishing, run the smallest relevant checks (`pnpm check`, `pnpm lint`, targeted tests) for confidence.
30+
6. Do not treat every principle as mandatory—some are OOP-oriented; apply judgment for TypeScript and Svelte.
31+
32+
## Output
33+
34+
- Review notes with principle numbers where helpful
35+
- Concrete refactor suggestions that respect `AGENTS.md` (no type assertions, no comments in product code unless the task explicitly allows documentation elsewhere, verb/noun naming, and so on)
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
# Code quality principles
2+
3+
Universal principles distilled from Kent Beck's Smalltalk Best Practice Patterns, generalized for any codebase. For **stack-specific commands, agent roles, and rules that conflict with these principles**, the nearest [`AGENTS.md`](../../../AGENTS.md) wins.
4+
5+
## 1. Composed Method
6+
7+
Divide every function into sub-functions that each perform one identifiable task. Keep all operations in a method at the same level of abstraction. This naturally produces many small methods, each a few lines long.
8+
9+
## 2. Intention-Revealing Names
10+
11+
Name methods and functions after what they accomplish, never how they accomplish it. A reader should understand the purpose of a call without reading its body.
12+
13+
## 3. Replace Comments with Clear Code
14+
15+
If a comment restates what the code does, delete it. If you cannot delete a comment, refactor the code (extract a well-named function, rename a variable) until the comment is redundant. Reserve comments for why, not what.
16+
17+
## 4. Constructor Clarity
18+
19+
Provide factory functions or constructors that create well-formed instances. Pass all required parameters upfront so callers never receive half-initialized objects.
20+
21+
## 5. Single Responsibility for Methods
22+
23+
Each method should have exactly one reason to change. If a method requires a paragraph to explain, it is doing too much.
24+
25+
## 6. Say Things Once and Only Once
26+
27+
Every piece of knowledge or logic should exist in exactly one place. Duplicate code is a multiple-update liability—extract it.
28+
29+
## 7. Behavior Over State
30+
31+
Get the behavior (public interface) right first. Internal representation can always change later if it is hidden behind a clean API. Optimizing data layout prematurely couples consumers to implementation details.
32+
33+
## 8. Intention-Revealing Selectors / Function Names
34+
35+
Name functions after the concept they represent, not the algorithm they use. `includes(item)` is better than `linearSearchFor(item)`. Imagine a second, very different implementation—would you give it the same name? If not, generalize.
36+
37+
## 9. Guard Clauses Over Deep Nesting
38+
39+
Handle edge cases and error conditions at the top of a function and return early. The main logic path should read without indentation.
40+
41+
## 10. Query Methods Return; Commands Mutate
42+
43+
Separate functions that answer questions (return a value, no side effects) from functions that change state. Name query methods with `is`, `has`, `can` prefixes for booleans.
44+
45+
## 11. Explaining Variables
46+
47+
When a complex expression is hard to read, assign its result to a well-named local variable. The variable name becomes the explanation.
48+
49+
## 12. Role-Suggesting Names
50+
51+
Name variables after the role they play, not their type. `employees` not `employeeList`; `query` not `queryString`. The type can be inferred from context.
52+
53+
## 13. Use Polymorphism Instead of Conditionals
54+
55+
When the same if/switch structure appears in multiple places, replace it with polymorphic objects that each implement one branch. Adding a new case becomes adding a new class, not editing existing code.
56+
57+
## 14. Delegate, Do Not Inherit (Prefer Composition)
58+
59+
Share implementation by passing work to a collaborator object rather than subclassing. Delegation keeps the two objects independently replaceable and avoids deep inheritance hierarchies.
60+
61+
## 15. Method Object for Complex Logic
62+
63+
When a method has grown huge and shares many temporaries, extract the entire computation into its own class. Turn parameters and temporaries into instance fields, put the logic in a `compute()`/`call()` method, then simplify with Composed Method.
64+
65+
## 16. Execute Around (Resource Bracketing)
66+
67+
When two actions must always happen together (open/close, lock/unlock, setup/teardown), expose a single function that accepts a callback. The caller never forgets the second action.
68+
69+
## 17. Explicit Initialization
70+
71+
Initialize all state at construction time. Never rely on callers to set fields in the right order after creation. If defaults exist, set them in the constructor.
72+
73+
## 18. Lazy Initialization
74+
75+
When computing or fetching a value is expensive and may not be needed, defer it to first access. Cache the result in a field and return it on subsequent calls.
76+
77+
## 19. Constant Methods / Named Constants
78+
79+
Replace magic literals with named constants or zero-argument methods. `MAX_RETRIES` communicates more than `5`, and a method lets subclasses override the value.
80+
81+
## 20. Indirect Variable Access (Encapsulate Fields)
82+
83+
Access instance fields through getter/setter methods rather than directly. This gives you a single place to add validation, logging, lazy init, or change notification later.
84+
85+
## 21. Collection Accessor Safety
86+
87+
Never return a raw mutable collection from a getter. Return a copy, an immutable view, or expose only domain-specific add/remove/enumerate methods.
88+
89+
## 22. Equality and Hashing Contract
90+
91+
If you override equality, override hashing to match. Objects that are equal must produce the same hash. Base both on the same set of fields.
92+
93+
## 23. Mediating Protocol
94+
95+
When two objects collaborate heavily, make the set of messages between them explicit and consistent. Name them coherently so a third party can implement the same interface.
96+
97+
## 24. Double Dispatch for Cross-Type Operations
98+
99+
When behaviour depends on the types of two objects (not just the receiver), use double dispatch: the receiver calls back the argument with a more specific method, including its own type in the name.
100+
101+
## 25. Pluggable Behaviour Over Subclass Explosion
102+
103+
When many subclasses differ in only one or two methods, replace the hierarchy with a single class that accepts a strategy (callback, lambda, or strategy object). Reserve subclassing for genuinely different families of behaviour.
104+
105+
## 26. Collecting Parameter
106+
107+
When multiple sub-methods need to contribute to a single result collection, pass the collection as a parameter rather than concatenating return values or stashing state in a field.
108+
109+
## 27. Interesting Return Values Only
110+
111+
A method should return a value only when the caller needs it. Do not return `self` or internal state by default—return something meaningful or nothing at all. Make return values intentional.
112+
113+
## 28. Reversing Method for Readable Flow
114+
115+
If sending messages to multiple receivers in sequence breaks readability, add a convenience method on the parameter so all calls flow through one object. Readable left-to-right flow matters.
116+
117+
## 29. Debug Printing for Developer Ergonomics
118+
119+
Override `toString`/`__repr__`/`inspect` to show the structural information a developer needs when debugging. User-facing display strings are a separate concern.
120+
121+
## 30. Adopt Patterns Incrementally
122+
123+
Do not try to apply all rules at once. Write code, notice friction, then apply the pattern that resolves it. Patterns are refactoring targets, not upfront mandates. Clean up as you go.
124+
125+
---
126+
127+
## How to use this file
128+
129+
Use it for code review, pairing, and AI-assisted development. Reference a principle by number (for example, "Principle 8: name after the concept, not the algorithm"). This file is the canonical list; [`AGENTS.md`](../../../AGENTS.md) defines project-specific rules and agent roles.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
name: code-quality-principles
3+
description: Cross-language code quality principles for review and refactors. Use when reviewing structure and naming, applying Kent Beck-style patterns, or when working with the code-quality dev agent. Canonical text lives under .agents/.
4+
---
5+
6+
# Code quality principles
7+
8+
The full numbered list is versioned in [.agents/dev/skills/code-quality-principles.md](../../../.agents/dev/skills/code-quality-principles.md). Read that file for all 30 principles and how to cite them.
9+
10+
The `code-quality` dev agent is defined in [.agents/dev/agents/code-quality.md](../../../.agents/dev/agents/code-quality.md). Stack-specific rules always come from the nearest [`AGENTS.md`](../../../AGENTS.md).

.cursor/skills/e2e-playwright-extension/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Aligned with [WXT’s Playwright E2E example](https://github.com/wxt-dev/example
99

1010
## Layout
1111

12-
- Tests in [e2e/tests/](e2e/tests/) — main suite [e2e/tests/extension.spec.ts](e2e/tests/extension.spec.ts) with describe blocks: **Popup**, **Content injection**, **Per-domain activation**
12+
- Tests in [e2e/tests/](e2e/tests/) — main suite [e2e/tests/extension.spec.ts](e2e/tests/extension.spec.ts) with describe blocks: **Popup**, **Content injection**, **Per-domain activation**; [e2e/tests/toolbar-settings.spec.ts](e2e/tests/toolbar-settings.spec.ts) covers toolbar settings menu (Feature Voting upvotes multi-on + isolation, behaviour toggles)
1313
- Shared fixture in [e2e/fixtures.ts](e2e/fixtures.ts)
1414
- Page helpers in [e2e/pages/](e2e/pages/) (e.g. [e2e/pages/popup.ts](e2e/pages/popup.ts))
1515
- Fixture page served via Playwright `webServer` + custom static server:

.cursor/skills/skill-hygiene/SKILL.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ When updating a skill (see `.cursor/rules/skill-maintenance.mdc` for when to do
3131
- **Fixture:** context/extensionId and MV3 vs background page still match [e2e/fixutes.ts](e2e/fixutes.ts).
3232
- **Run:** `pnpm test:e2e` and build-before-test still correct.
3333

34+
## code-quality-principles
35+
36+
- Thin Cursor skill points at [.agents/dev/skills/code-quality-principles.md](../../../.agents/dev/skills/code-quality-principles.md); if that path or agent roles change, update the skill and [AGENTS.md](../../../AGENTS.md).
37+
3438
## After editing
3539

3640
- Keep SKILL.md under 500 lines; move long detail to reference.md if needed.

.cursor/skills/wxt-svelte-extension/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ description: Provides project context for the amgiflol WXT + Svelte 5 browser ex
77

88
## Stack
99

10-
- WXT, Svelte 5 (runes), UnoCSS
10+
- WXT, Svelte 5 (runes), UnoCSS, `@lucide/svelte` v1 for icons
1111
- Entrypoints in [src/entrypoints/](src/entrypoints/) (e.g. background.ts, content.ts)
1212
- Config in [wxt.config.ts](wxt.config.ts); srcDir `src`, outDir `dist`
1313

AGENTS.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,16 @@ High-level agents for this project:
6565
- Focus: Svelte 5 + WXT extension code (popup, background, settings, stores).
6666
- Reads: this file, Svelte/WXT rules, and relevant e2e sections.
6767
- Must keep diffs small, update tests when behavior changes, and ensure `pnpm check`, `pnpm lint`, and relevant tests pass.
68+
- **Dev agent (code quality) – `code-quality`**:
69+
- Focus: Review, refactors, naming, and structure using `.agents/dev/skills/code-quality-principles.md`.
70+
- Reads: that file, `.agents/dev/agents/code-quality.md`, the nearest `AGENTS.md`, and relevant code.
71+
- When to use: see **Choosing dev agents** below.
72+
73+
### Choosing dev agents
74+
75+
- **`typescript-pro`**: New features, bug fixes that change behaviour, wiring stores/popup/background, and any work whose main deliverable is implementation in `src/`.
76+
- **`code-quality`**: PR-style review, refactors driven by structure/naming/decomposition, applying the numbered principles in `.agents/dev/skills/code-quality-principles.md`, cleanup without a feature spec, or when the user asks for a quality or review pass.
77+
6878
- **QA agent – `test-automator`**:
6979
- Focus: Playwright e2e tests and fixtures under `e2e/`.
7080
- Reads: testing and e2e sections here plus any nested `AGENTS.md` inside `e2e/` (if present).

CHANGELOG.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@
131131

132132
- 9e2516f: unblock triggering of shorcuts while toolbar is focused
133133
- d75a508: update tracker on scroll and window resize
134-
135134
1. Fixes [#12](https://github.com/sm17p/amgiflol/issues/12)
136135
2. Partial fix [#7](https://github.com/sm17p/amgiflol/issues/7)
137136

CLAUDE.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,7 @@
33
This project uses the `AGENTS.md` standard as the primary source of instructions for coding agents.
44

55
- Read the nearest `AGENTS.md` before making changes.
6-
- Follow the agent roles described there (`extension-dev`, `e2e-qa`, `docs-maintainer`, `release-manager`) instead of inventing new ones.
6+
- Follow the agent roles described there (`typescript-pro`, `code-quality`, `test-automator`, `docs-maintainer`, `release-manager`) instead of inventing new ones.
77
- When `CLAUDE.md` and `AGENTS.md` conflict, prefer the guidance in `AGENTS.md`.
8+
9+
Cross-language code quality principles (optional context, loaded on demand): @.agents/dev/skills/code-quality-principles.md

LICENSE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
MIT License
1+
# MIT License
22

33
Copyright (c) 2025 Smit Patel
44

0 commit comments

Comments
 (0)