Repository navigation
Release 1.1.0: header icon actions, pin fixes, test design policy and E2E in CI - #10
Merged
Merged
Conversation
Vite + React, Vite + Vue, Astro 5, Next.js 16 and plain HTML apps join the pnpm workspace, link needmark with workspace:*, and start with pnpm examples:<name>. The package-manager check now installs the tarball into a copy of examples/vite-react and starts its dev script per runner. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review UI
- DOM capture compares iframe geometry in document coordinates, so a
page with smooth scrolling and an embedded map no longer fails.
- A failed capture falls back to an image-free save instead of
blocking the comment behind a separate button.
- The first comment starts selection without opening settings.
- Remove the selected-text comment button and multiple-selection page
operation.
- Keep the hidden category radios inside their labels, which removed
the composer's horizontal scroll.
- The list header copy button shows "Copied" text, and the guide link
in the draggable toolbar opens again.
Astro
- The Vite plugin appends the review script to HTML that a framework
renders without transformIndexHtml, so `astro dev` works with
`vite: { plugins: [needmark()] }`. The proxy shares the tag builder.
init
- Dialogue, output and flag errors are in English.
- Offer the recommended settings first; declining asks each choice as
a numbered list that explains it in plain terms.
- The generated config explains the language and capture values.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copied unchanged from the main checkout so the following prompt fixes show as their own diff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
needmark 1.0.1 on npm never injects the toolbar into Astro pages through the Vite plugin, so the prompt now treats Astro like Next.js. It also tells the agent how to pick another review port when 4190 is taken. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The English and Japanese pages explain installation and daily use, and lead with a prompt that a coding agent can follow to install NeedMark, initialize it and connect the development server. A Pages workflow deploys site/ from main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep the draft's note that Astro loads the plugin through vite.plugins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Astro dev now gets the widget from needmark() in vite.plugins, so the example no longer needs the review proxy or concurrently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The setup prompt now loads NeedMark into Astro through vite.plugins instead of the review proxy, and the manual section explains that init offers recommended settings first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
next dev regenerates tsconfig.tsbuildinfo, so it belongs with the other ignored example output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After `pnpm add -D needmark` and `needmark init`, the usual `pnpm dev` now shows the toolbar. - init recommends connecting the development server and, with consent, adds needmark() to vite.config.* or astro.config.*, or prefixes a Next.js dev script with `needmark dev --`. It prints each changed line, leaves configs it cannot read untouched, and changes nothing outside its own files without a terminal unless --connect=yes is given. Run again in a configured project, it keeps needmark.config.ts and only offers the connection. - `needmark dev -- <command>` serves the review proxy on the app's usual port and starts the command on a free one through PORT. - NEEDMARK=off (per person or run) and `enabled: false` (shared) switch NeedMark off; the plugin then adds nothing and the wrapper only runs the command. - `needmark remove` undoes the connection and keeps the config and feedback. - The guides, README and site describe the three-step setup, the Next.js example uses the wrapper, and the package-manager check removes and reconnects the plugin through each runner. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment details now show the prompt copy and editor links as header icon buttons; a successful copy turns the icon into a success-coloured check in both the details and the list. Panels are narrower with smaller titles and names, the composer and settings drop their headers in favour of a Cancel/Save footer, and the list badge overlaps the icon's lower right. Pins no longer follow page DOM changes while the pointer is on one, so a page that reacts to the pointer leaving it (a chart hiding its tooltip) cannot move a pin away mid-click. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Specs now import test/expect from tests/support/review.ts, whose automatic fixture empties the shared feedback folder before every test, instead of each file declaring its own output path and cleanup (several forgot it). Repeated settings changes and served-attribute rewrites become configureReview and enableWidgetOption, and temporary paths come from one tempRoot so the suite is no longer tied to macOS's /private/tmp. Only typecheck and a Chromium subset (review-ux, handoff-actions, cli-onboarding) were run; the full suite follows with the remaining test design work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Details show a single target's outline on the page, but leaving them only removed the outlines of multiple-selection groups, so the blue outline stayed after going back to the list, closing the panel or deleting the comment, and therefore also through Resolve all and Delete all. clearLocated now hides the shared outline whenever it marks a located target, leaving a drag preview untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs/testing.md now states the principles every test follows (public interfaces, smallest layer that catches the failure, one behaviour per test, failing test first for bugs, isolation without fixed waits), the small/medium/large layers, what not to test, how to write tests with tests/support, how to run them and how to keep the suites small, with the sources these rules follow. Chromium runs every spec; WebKit repeats only tests tagged @cross-engine, whose outcome depends on engine behaviour, plus one review-flow smoke test. A full E2E run drops from 440 to 300 tests and from about 13 to 8 minutes. Styling-only checks (sticky header, panel overflow, success colour) are removed and two overlapping selection tests are merged. AGENTS.md lists procedures and their skills, CONTRIBUTING.md and the architecture reference point to the policy, and a new testing skill gives agents the step-by-step procedure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add an E2E job to ci.yml on Node 24 that installs Chromium and WebKit, runs pnpm test:e2e under xvfb-run because Chromium is headed, and uploads test-results/ when it fails. Three assumptions only held on macOS: - Chromium on Linux reserves a classic scrollbar, so the viewport was 1185 px wide; Chromium now runs with overlay scrollbars everywhere. - Linux draws a near-black focus ring that the iframe ink measurement counted as text; the note is blurred before measuring. - Native tab capture colours drift up to 16 per channel on Linux, so those checks use a shared platform-specific tolerance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The dev proxy listens before the wrapped command does and answers 502 until then. The test stopped polling at that first response, which failed the Node 24 CI job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Save is enabled as the capture preview appears, while the panel is still morphing to fit it. On the slower CI runner, Playwright saw the button stable mid-animation and the click landed above it, so nothing was saved. The save helpers now wait until the panel loses its morphing class. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Save was enabled as soon as the capture preview appeared, while the panel was still morphing to fit it, so a click aimed at the button could land where it had been a frame earlier. Enable it only after the morph has ended. Because an animation's finished promise resolves before its finish event, the wait settles the morph itself. This replaces the test-side waits from the previous commit, and a new panel-position test checks that Save is enabled only after the panel settles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…apture html2canvas lays its clone out with scrolling="no", so on pages whose viewport reserves room for classic scrollbars the clone was wider than the live page and the iframe parent check rejected the capture. The clone now keeps the page's own scrollbars. A cross-engine spec styles the scrollbar to cover this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Export runtimeFramesOf from code-context.ts and test V8 and JavaScriptCore stack parsing in Vitest, leaving the browser test for fiber selection. Delete or merge browser tests that checked the same rule twice (review-ux, multi-complete, frame-padding, feedback, context-attributes). E2E runs drop from 300 to 293; Vitest grows from 508 to 512. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uttons Following the design file, buttons with the danger class (the delete confirmations) use a #ffe9e8 background with #680000 text in both themes, while danger items in kebab menus keep the themed text colour. Primary buttons and the hint toolbar buttons no longer use a semibold label. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…heck The response wait was registered before page.reload, so it could match the old page's two-second poll that answered after navigation started; its body is gone by then and Playwright fails with "No resource with given identifier found". Only responses that arrive after the reload commits are accepted now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…llapse button A chart that adds tooltip text while the pointer is over it made its pin fall back to the recorded position. While the pointer is on a target that passed the strict identity check on the previous render, a change to only its text or accessible name keeps the pin on the same node, provided the recorded selector still resolves to it and every other identity check still passes; once the pointer leaves, the strict check applies again. The toolbar's collapse button now shows the NeedMark logo, blue in the light theme and white in the dark theme. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On the slower CI runner the saved comment reached the page after the first measurement, when the pin was attached but not yet laid out, and the non-null assertion threw instead of letting the poll retry. The test now waits for the pin to be visible and treats a missing box as not yet settled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Releases needmark 1.1.0. Merging this into
maintriggers the Release workflow (npm publish, tag and GitHub releasev1.1.0).UI
#ffe9e8/#680000) in both themes; button labels use the regular weight.Fixes
Tests and process
docs/testing.mddefines the test design policy (smallest layer that catches the failure, one behaviour per test, no styling assertions); atestingskill gives agents the procedure.@cross-enginetests. Shared E2E setup lives intests/support; stack parsing moved to Vitest.Verification
pnpm build,pnpm typecheck,pnpm test(512),pnpm test:e2e(295) andpnpm test:package-managers(npm, pnpm, Yarn 4, Yarn 1) pass locally on macOS.🤖 Generated with Claude Code