Conversation
Vite + React + TypeScript notebook UI demonstrating mutable document CRUD on Dash Platform: register a note contract, then create, edit, and delete notes against it via the shared evo-sdk core. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Same-id updates skipped the detail refetch because setSelectedId with an unchanged value didn't re-run the load effect, leaving stale revision / updatedAt and a stuck dirty state. Extract loadNoteDetail and call it directly from handleSave. Suppress the loading placeholder when the already-loaded data still matches the current selection so the editor form and notes list don't unmount during background refreshes. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…eight Both panels previously sized to their content, so heights mismatched and jumped between notes. At xl breakpoint the grid wrapper now claims a fixed viewport-relative height; both sections fill it via flex-column, and the editor textarea expands into remaining space. Mobile layout is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Surface the platform's per-field byte limit in the editor with a live counter above the body and a disabled Save button when over. Also truncate the header title preview to a single line and drop the "Apple Notes-like" callout. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Replace the previous note's stale title with a pulsing "Loading…" state while a newly selected note is fetched, and unwrap the now-single-child header flex row. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
On mobile, the workspace now switches to a list/editor stack with a back button, fullscreen editor, search bar, compose FAB, and a centered empty-state CTA when signed out. The list and editor panes paint edge-to-edge on a shared surface so previously distinct cards no longer read as disjoint blocks. Desktop two-pane layout is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds a parameterizable matchMedia stub and exercises the mobile-only back/compose/delete flows, the desktop-vs-mobile auto-select gate, the NoteList search filter, the no-contract EmptyState branch, and the Bridge identity link on the auth-gating EmptyState. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…nter The tutorial header above the workspace pushed the EmptyState's geometric center below the screen's visual center on mobile. Translate the centered cluster up by 64px so the sign-in CTA sits closer to where the eye expects it. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Persists only the most recently logged-in identity ID so returning visitors automatically resume in a read-only browsing view of their notes. Adds a "browsing" session status, account-style login UX with a read-only identity field plus switch/forget links, and mirrors the same controls in the authenticated settings view. Mnemonic and keys remain in-memory only. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Adds tests for the previously uncovered LoginModal flows: Cancel button, switch-then-submit (rememberMe stays on), state reset on modal reopen, and the settings-view Use-different-identity, Forget, Close, and Logout-side-effect paths. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…alidate Persists per-identity note lists (titles, bodies, revisions) so reloads paint instantly from cache, then revalidate against Platform in the background. Authenticated saves are gated on first revalidation completing so cached state can't clobber a newer chain revision; conflicts during edits surface a warning rather than silently overwriting. Refreshing indicator in the list header during revalidation; visibilitychange + 30s interval drive mid-session freshness, with throttling and write-in-flight guards. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…s warnings After a failed update, refresh from chain and surface the conflict warning if the revision moved past what was loaded — so a retry isn't a silent overwrite when another window saved first (typically exposed via an identity nonce error). The warning supersedes the raw error since it's the actionable signal that a retry will overwrite. Also advance the baselines synchronously after a successful save so the post-save reload's read-time conflict detector doesn't false-positive against its own write. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Wraps the My notes list and Note editor in one card with an internal divider on desktop, moves the title into the editor header as an editable heading with hover affordance, and consolidates byte count into the metadata footer. Both header rows share a fixed height so the bottom borders align across the divider. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
JSDOM ignores CSS, so md:hidden left both title inputs in the accessibility tree and getByLabelText returned multiple matches. Gate the desktop header input and mobile body input on the existing isDesktop signal so exactly one is mounted at a time. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
… identity Boot in browsing mode when an identity is remembered so the signed-in gate, "No notes yet," and "No note selected" no longer flash through before the cache hydrates. NotesWorkspace seeds notes (and the desktop editor pane) synchronously from localStorage, and reloadNotes waits for the SDK instead of wiping cached state during rehydrate. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ical path Defer the @dashevo/evo-sdk import (and its ~8MB WASM bundle) until the user actually connects or logs in, and strip the SDK chunk from Vite's auto-injected modulepreload hints so the browser doesn't race to fetch it during initial paint. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…act change Every note query and fetch was calling refreshContractCache, which evicts the SDK's cached contract schema and forces a refetch on every documents operation. The contract is immutable per session, so this was costing a round-trip per note read with no benefit. Drop the per-query eviction and instead evict from setContractId only when the contract ID actually changes (settings update or fresh register). Add SessionContext tests covering both branches. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Renames the directory, package name/displayName, HTML title, README, log strings, and the four localStorage keys (patchbook-lab.* → dashnote.*). Existing browser sessions lose remembered contract, identity, cached notes, and theme — acceptable for a testnet example. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Move the title input out of the header bar into the content area as the first line of a borderless editor surface, and drop the search row's divider so panes don't compete with the shell border. Wraps the global input/textarea font reset in @layer base so utility font sizes can win. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The two-column shell only applied at xl, so 768-1279px viewports rendered the list and editor stacked vertically as separate cards. Move the shell breakpoint to md and use a 260px list column at md, growing to 340px at lg+, so tablets get the same Apple Notes layout as larger screens. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Bytes vs characters has no fixed conversion (emoji are 4 bytes each), so 'X / 5120 bytes' didn't tell users how much room they had left. Show a thin progress bar that fills against the 5120-byte limit, turning amber at 90% and red when oversize. Bar appears at 75%+ and stays hidden otherwise; precise byte count surfaces via the title tooltip and ARIA valuetext. Also reorder the desktop footer to lead with Revision and drop the redundant relative-time parenthetical from Updated. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Move non-SDK utilities (logger, notesCache, rememberedIdentity) out of src/dash/ into src/lib/, move useMediaQuery into src/hooks/, and add "SDK method:" JSDoc headers to each src/dash/ operation file so the folder reads as a tour of Platform calls. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Mirror the dashmint-lab and dashproof-lab README structure (prerequisites, ops table mapping action → file → SDK method, reading-order tour, tech stack) and add a CLAUDE.md dev guide covering architecture, SDK patterns, and contract gotchas. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Resolve `sdk.dpns.username(identityId)` after login and persist the (id, name) pair to localStorage so a returning visitor sees their name in the sidebar, settings, and remembered-identity panels without re-querying. DPNS bindings are immutable, so cached pairs are never revalidated. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Mirrors dashmint-lite / dashproof-lite: zero-build static HTML loading the Evo SDK from esm.sh. Recent notes (with optional owner filter), get note by ID, click-to-copy on truncated identifiers. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Merge the two list functions and number coercers, drop redundant epoch fields in favor of ISO-only timestamps (lex-sortable), and inline the shortId helper into copyIdSpan. 427 → 394 lines, behaviour unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdded a complete Dashnote example app (React + TypeScript + Vite) implementing read-only browsing and authenticated create/update/delete flows against a Dash Platform note contract, plus SDK wrappers, session management, caching, UI components, tests, and documentation. ChangesDashnote Example App
Sequence Diagram(s)sequenceDiagram
participant Browser
participant SessionProvider
participant IdentityKeyManager
participant EvoSDK
participant LocalStorage
Browser->>SessionProvider: mount
SessionProvider->>LocalStorage: loadRememberedIdentity(), loadStoredContractId()
SessionProvider->>EvoSDK: dynamic import -> createClient()
SessionProvider->>Browser: provide SessionContext
Browser->>SessionProvider: user submits mnemonic (login)
SessionProvider->>IdentityKeyManager: IdentityKeyManager.create(mnemonic, index)
IdentityKeyManager-->>SessionProvider: keyManager (identityId, getAuth)
SessionProvider->>LocalStorage: saveRememberedIdentity(identityId)
SessionProvider->>EvoSDK: ensure connected client (if needed)
SessionProvider->>Browser: status "authenticated"
Browser->>SessionProvider: create/update/delete note
SessionProvider->>IdentityKeyManager: getAuth()
IdentityKeyManager-->>SessionProvider: auth (identityKey, signer)
SessionProvider->>EvoSDK: sdk.documents.create/replace/delete(...)
EvoSDK-->>SessionProvider: operation result
SessionProvider->>LocalStorage: saveCachedNotes(updatedNotes)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (9)
example-apps/dashnote/src/components/NavButton.tsx (1)
22-28: ⚡ Quick winAdd
aria-hidden="true"to the glyph<span>to prevent redundant screen reader announcements.Without it, a screen reader will announce the glyph character (e.g., "pencil" for ✏) in addition to the
labeltext, producing a combined name like "pencil Notes".♿ Proposed fix
<span + aria-hidden="true" className={`w-3.5 text-center text-sm ${ active ? "text-accent" : "text-ink-4" }`} > {glyph} </span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/src/components/NavButton.tsx` around lines 22 - 28, The glyph <span> inside the NavButton component is being read by screen readers alongside the label, causing redundant announcements; update the glyph span (the element rendering {glyph} in NavButton.tsx) to include aria-hidden="true" so the decorative glyph is ignored by assistive tech while keeping the visible label accessible.example-apps/dashnote/package.json (1)
20-24: ⚡ Quick winMove
tailwindcssand@tailwindcss/vitetodevDependencies.Both are build-time tools with no browser runtime presence — they're consumed solely during
vite buildandvite dev. Placing them independenciesbloats the install footprint for any consumer of this package and misrepresents the runtime surface.♻️ Proposed fix
"dependencies": { "@dashevo/evo-sdk": "3.1.0-dev.1", - "@tailwindcss/vite": "^4.2.2", "react": "^19.2.4", "react-dom": "^19.2.4", - "sonner": "^2.0.7", - "tailwindcss": "^4.2.2" + "sonner": "^2.0.7" }, "devDependencies": { + "@tailwindcss/vite": "^4.2.2", + "tailwindcss": "^4.2.2", "@eslint/js": "^9.39.4",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/package.json` around lines 20 - 24, The package.json currently lists "tailwindcss" and "@tailwindcss/vite" under dependencies; move these two entries into devDependencies so they are only installed for build/dev time. Edit package.json to remove the "tailwindcss" and "@tailwindcss/vite" keys from the dependencies object and add them with the same version strings under devDependencies, leaving runtime deps like "react" and "react-dom" unchanged.example-apps/dashnote/test/queries.test.ts (1)
16-46: ⚡ Quick winAdd coverage for the plain-object result shape in
normalizeNotes.The current
normalizeNotessuite only exercises theMappath. The plain-object branch (theelseatqueries.tslines 75-79) is untested, yet the guideline requiresnormalizeNotesto handle all three shapes (array, Map, plain object). A regression there would be silent.✅ Suggested additional test case inside the `normalizeNotes` describe block
describe("normalizeNotes", () => { it("normalizes arrays, maps, and revision values", () => { // ... existing Map test ... }); + + it("normalizes a plain-object result", () => { + const notes = normalizeNotes({ + "note-2": { + $ownerId: "owner-2", + $createdAt: 500, + $updatedAt: 600, + $revision: 1, + title: "Plain", + message: "Object path", + }, + }); + expect(notes).toEqual([ + { + id: "note-2", + ownerId: "owner-2", + title: "Plain", + message: "Object path", + createdAt: 500, + updatedAt: 600, + revision: 1, + }, + ]); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/test/queries.test.ts` around lines 16 - 46, Add a new test case in the normalizeNotes describe block that exercises the plain-object input branch: call normalizeNotes with a plain object (e.g., { "note-1": { $ownerId: "...", $createdAt: "1000", $updatedAt: 2000n, $revision: "4", title: "...", message: "..." } }) and assert it returns the same normalized array shape as the existing Map test (id, ownerId, createdAt as number, updatedAt as number, revision as number, title, message); this ensures the plain-object branch in queries.ts (the else path) is covered and handles types like bigint and string revisions correctly.example-apps/dashnote/test/dash.test.ts (1)
88-123: ⚡ Quick winAdd a test for the "note not found" error branch in
updateNote.
updateNote.tsthrowsError("Note … not found.")whensdk.documents.getreturns a falsy value (lines 36-38), but this branch has no test coverage. It's a real production path (note deleted between open and save), and the throw propagates to the caller's error handler.✅ Suggested addition inside the `updateNote` describe block
describe("updateNote", () => { it("fetches the current note and increments revision before replace", async () => { // ... existing test ... }); + + it("throws when the note is not found", async () => { + const sdk = { + documents: { + get: vi.fn().mockResolvedValue(null), + replace: vi.fn(), + }, + }; + await expect( + updateNote({ + sdk: sdk as never, + keyManager: makeKeyManager() as never, + contractId: "contract-1", + noteId: "note-missing", + message: "body", + }), + ).rejects.toThrow("Note note-missing not found."); + expect(sdk.documents.replace).not.toHaveBeenCalled(); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/test/dash.test.ts` around lines 88 - 123, Add a new test in the updateNote describe block that covers the "note not found" branch by mocking sdk.documents.get to resolve to a falsy value (e.g., null), calling updateNote with the same minimal args used in existing tests, and asserting that the call rejects/throws the exact Error message produced by updateNote (the "Note … not found." message); also ensure mockDocumentConstructor is reset and sdk.documents.replace is not called in this test so you validate the early exit path.example-apps/dashnote/src/components/NoteList.tsx (1)
61-81: ⚡ Quick winDuplicate spinner SVG — consider extracting a shared
<Spinner>component.The same four-element SVG (two
<circle>tracks +<path>arc) appears verbatim twice, differing only in theclassName. Any future tweak to the spinner shape will need to be applied to both.♻️ Suggested extraction (can live at the top of this file or in a shared `src/components/` file)
+function Spinner({ className }: { className: string }) { + return ( + <svg className={className} viewBox="0 0 24 24" fill="none" aria-hidden="true"> + <circle + cx="12" cy="12" r="10" + stroke="currentColor" strokeOpacity="0.25" strokeWidth="3" + /> + <path + d="M22 12a10 10 0 0 1-10 10" + stroke="currentColor" strokeWidth="3" strokeLinecap="round" + /> + </svg> + ); +}Then replace both usages:
- <svg - className="h-3 w-3 animate-spin" - viewBox="0 0 24 24" - fill="none" - aria-hidden="true" - > - <circle cx="12" cy="12" r="10" stroke="currentColor" strokeOpacity="0.25" strokeWidth="3" /> - <path d="M22 12a10 10 0 0 1-10 10" stroke="currentColor" strokeWidth="3" strokeLinecap="round" /> - </svg> + <Spinner className="h-3 w-3 animate-spin" />- <svg - className="h-7 w-7 animate-spin text-ink-4" - viewBox="0 0 24 24" - fill="none" - aria-hidden="true" - > - <circle cx="12" cy="12" r="10" stroke="currentColor" strokeOpacity="0.25" strokeWidth="3" /> - <path d="M22 12a10 10 0 0 1-10 10" stroke="currentColor" strokeWidth="3" strokeLinecap="round" /> - </svg> + <Spinner className="h-7 w-7 animate-spin text-ink-4" />Also applies to: 133-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/src/components/NoteList.tsx` around lines 61 - 81, The spinner SVG is duplicated in NoteList.tsx; extract it into a reusable Spinner component (e.g., function Spinner({className}: {className?: string}) or a React.FC<{className?: string}>) that renders the SVG (preserve viewBox, aria-hidden, circle and path elements) and accepts a className prop for the differing sizes/animations, then replace both inline SVGs in the NoteList component with <Spinner className="..."/> (or the equivalent JSX) and export/move Spinner to a shared components location if desired.example-apps/dashnote/src/styles/globals.css (1)
4-4: ⚡ Quick winConfigure Stylelint to recognize Tailwind v4 at-rules.
The
scss/at-rule-no-unknownrule flags@themeas unknown — a false positive for Tailwind v4's CSS-first configuration, which introduces@theme,@source, and@utility. Without suppression this will cause lint errors for every Tailwind v4 directive.Add the directives to the
customSyntax/ ignore list in the Stylelint config:// .stylelintrc.cjs (or equivalent) { rules: { "scss/at-rule-no-unknown": [true, { ignoreAtRules: ["theme", "source", "utility", "layer"] }] } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/src/styles/globals.css` at line 4, Stylelint is flagging Tailwind v4 at-rules like `@theme/`@source/@utility as unknown; update the stylelint configuration to allow these by adding them to the scss/at-rule-no-unknown ignore list (add "theme", "source", "utility", and "layer" to ignoreAtRules for the rule "scss/at-rule-no-unknown") so Tailwind v4 CSS-first directives are accepted; modify your project stylelint config (e.g., .stylelintrc) to include that ignoreAtRules array.example-apps/dashnote/test/SessionContext.test.tsx (3)
243-245: ⚡ Quick winUnawaited
actcalls may silently miss async state updates.
act(() => { ref.current.forgetIdentity(); }),act(() => { ref.current.logout(); }), and similar calls are notawait-ed. If any of these handlers internally enqueue a microtask (e.g., aPromise.resolve()chain or asyncsetState), the assertions immediately following will run against stale state. React Testing Library recommends awaiting allactcalls even for nominally synchronous handlers.🔧 Proposed fix
- act(() => { - ref.current.forgetIdentity(); - }); + await act(async () => { + ref.current.forgetIdentity(); + });Apply the same pattern to every unwaited
actblock (lines 274–276, 293–295, 349–351, 384–386).Also applies to: 274-276, 293-295, 349-351, 384-386
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/test/SessionContext.test.tsx` around lines 243 - 245, The failing tests call act synchronously for handlers like ref.current.forgetIdentity() and ref.current.logout(), which can miss async state updates; update each unwaited act call (the blocks invoking ref.current.forgetIdentity(), ref.current.logout(), and other ref.current.* handlers at the highlighted locations) to be awaited—i.e., change act(() => { ... }) to await act(async () => { ... }) for every occurrence (lines flagged in the review: the calls around forgetIdentity, logout, and the other referenced blocks) so any microtasks or Promise-based state changes are flushed before asserting.
26-34: 💤 Low value
resolveDpnsNamemodule is not mocked — DPNS calls rely solely on the client stub.
mockDpnsUsernameis wired asclient.dpns.usernameon the mock SDK client but the../src/dash/resolveDpnsNamemodule is not intercepted withvi.mock. IfSessionContextimports and callsresolveDpnsName(sdk, id)directly, the real implementation runs and invokessdk.dpns.usernameon the mock client — this works. However, ifresolveDpnsNamehas any additional logic (caching, error handling, normalization) that the tests exercise implicitly, the tests cover more than the session layer alone. Conversely, if the module import path ever changes, the mock client stub silently stops exercising DPNS resolution.Explicitly mocking the
resolveDpnsNamemodule (similar to howrefreshContractCacheis mocked) would make the SessionContext tests a true unit test of only the session layer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/test/SessionContext.test.tsx` around lines 26 - 34, The tests currently rely on the mock SDK client for DPNS resolution but do not intercept the ../src/dash/resolveDpnsName module; add an explicit vi.mock for "../src/dash/resolveDpnsName" that returns a mocked resolveDpnsName implementation (e.g., returning mockDpnsUsername or resolving the same value the client.dpns.username stub provides) so SessionContext tests only exercise the session layer; update the mock to replicate any expected normalization/caching behavior used by SessionContext and ensure the mock is imported/used alongside the existing mockRefreshContractCache mock.
49-49: ⚡ Quick win
REMEMBERED_KEYis duplicated from production source.Hardcoding
"dashnote.lastIdentity"couples the test to an internal implementation detail. A rename of the key inrememberedIdentity.tswould silently make every localStorage assertion pass against the wrong key, masking the regression.Consider re-exporting the constant from
rememberedIdentity.ts(or a shared constants module) and importing it here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@example-apps/dashnote/test/SessionContext.test.tsx` at line 49, Replace the hardcoded REMEMBERED_KEY string in the test with the real constant exported from the production module: stop defining const REMEMBERED_KEY = "dashnote.lastIdentity" and instead import the exported constant (e.g., REMEMBERED_KEY or REMEMBERED_IDENTITY_KEY) from rememberedIdentity.ts (or a shared constants module) so tests follow the single source of truth; update the test import and remove the local const to reference that exported symbol in assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@example-apps/dashnote/src/components/AppShell.tsx`:
- Around line 133-138: The drawer toggle button in AppShell always uses
aria-label="Open menu" even when drawerOpen is true; update the button (the
element using onClick={() => setDrawerOpen(!drawerOpen)} and the drawerOpen
state) to conditionally set the aria-label (and optional title) to "Close menu"
when drawerOpen is true and "Open menu" when false so assistive tech announces
the correct action.
In `@example-apps/dashnote/src/components/LoginModal.tsx`:
- Around line 34-39: When the LoginModal reopens it must clear stale
authentication state: update the useEffect that runs on open inside LoginModal
to also call setError(null) and setMnemonic("") (alongside setRememberMe(true)
and setUseDifferentIdentity(false)) so error and mnemonic are reset whenever
open becomes true; locate the useEffect in LoginModal and add those two state
resets (using the existing setError and setMnemonic functions) to ensure no
stale error or typed mnemonic persists across open/close cycles.
In `@example-apps/dashnote/src/components/Modal.tsx`:
- Around line 37-55: The Modal currently focuses dialogRef on open but doesn't
trap keyboard focus; implement a focus-trap inside the Modal component: on open
(in the useEffect that watches open) capture document.activeElement as the
opener to restore later, query and store all focusable elements within
dialogRef, and add a keydown handler on dialogRef that intercepts Tab/Shift+Tab
to move focus from last->first and first->last respectively; remove the handler
and restore focus to the saved opener on close (onClose cleanup). Ensure the
handler is attached to the element referenced by dialogRef (the element with
role="dialog") so clicks still stopPropagation and other behavior (onClose)
remains unchanged.
In `@example-apps/dashnote/src/components/NotesWorkspace.tsx`:
- Around line 132-225: reloadNotes can apply a late async response to a
different session; capture a monotonic token or snapshot of session values
(e.g., const callToken = Symbol() or const started = { identityId, contractId,
status } ) before awaiting listMyNotes and then, immediately after the await
(before mutating state or calling saveCachedNotes), verify the current session
still matches the captured token/values (compare identityId, contractId, and
status or the token stored on a ref like reloadTokenRef.current) and bail out if
they differ; update references to reloadNotes, listMyNotes, identityId,
contractId, status, notesRef, and saveCachedNotes so the check is performed and
late responses are ignored.
In `@example-apps/dashnote/src/components/OperationResultNotice.tsx`:
- Around line 27-28: The component OperationResultNotice currently sets role
based on tone (role={tone === "error" ? "alert" : "status"}) but forces
aria-live="polite", which overrides the implicit assertive behavior of
role="alert"; fix by removing the hardcoded aria-live attribute or conditionally
setting aria-live="assertive" when tone === "error" so that role="alert" remains
assertive (leave role logic in OperationResultNotice unchanged, just remove or
change the aria-live usage).
In `@example-apps/dashnote/src/dash/contract.ts`:
- Around line 63-69: saveContractId and clearStoredContractId currently call
localStorage.setItem/removeItem directly which can throw and cause
registerContract to reject after the contract was created; wrap the body of both
functions in try/catch blocks, catch any DOM/storage exceptions, log or noop the
error but do not rethrow, and keep the functions' signatures so storage failures
don't bubble up and cause duplicate registrations (apply same change to the
other occurrence around line 134).
In `@example-apps/dashnote/src/hooks/useTheme.ts`:
- Around line 7-14: getInitialTheme currently calls
window.matchMedia?.("(prefers-color-scheme: light)").matches which can throw
when matchMedia is undefined because .matches is accessed without optional
chaining; update the check in getInitialTheme to safely handle missing
matchMedia (e.g., use window.matchMedia?.("(prefers-color-scheme:
light)")?.matches or first guard typeof window.matchMedia !== "function") so
that when matchMedia is absent the function falls back to returning "dark" for
STORAGE_KEY lookup failures.
In `@example-apps/dashnote/src/lib/format.ts`:
- Around line 11-27: The timestamp formatters treat 0 as falsy and return
"Pending"; change the conditional in formatTimestamp and formatCompactTimestamp
to check for null/undefined only (use timestamp == null or timestamp === null ||
timestamp === undefined) so epoch (0) prints correctly, and apply the same
nullish check to the other timestamp check referenced around lines 46-50 (update
the same function/variable there) to avoid conflating 0 with missing values.
In `@example-apps/dashnote/src/lib/notesCache.ts`:
- Around line 25-27: The cache key is currently built only from identityId in
storageKey(identityId: string) using STORAGE_PREFIX, so change the key format to
include contractId and network (e.g., STORAGE_PREFIX + identityId + '|' +
contractId + '|' + network) and update all cache helper functions that call
storageKey (the get/set/clear helpers referenced near lines 29-33, 36-37, 46-47,
60-65, 82-86) to accept contractId and network parameters and pass them into the
new storageKey signature so each identity+contract+network combination gets its
own isolated storage key.
In `@example-apps/dashnote/src/session/SessionContext.tsx`:
- Around line 164-175: The DPNS lookup (resolveDpnsName) should not drive the
session into an error state: wrap the await resolveDpnsName(connected,
resolvedId) call in a try/catch, and on rejection set resolvedName = null and
call setDpnsName(null) (or skip setting) instead of throwing—then continue the
normal flow (including saveRememberedIdentity and setRememberedIdentityId) so
the session remains valid; apply the same try/catch pattern to the other
occurrence of resolveDpnsName in this file so DPNS failures are isolated and do
not mark the provider as "error".
- Around line 172-175: When handling login where rememberMe is false, clear any
previously persisted identity instead of leaving it intact: in the same block
that currently calls saveRememberedIdentity and setRememberedIdentityId when
rememberMe && resolvedId, add the opposite branch for !rememberMe to call the
cleanup functions (e.g., invoke saveRememberedIdentity with a cleared
value/removed identity and call setRememberedIdentityId(null) or equivalent) so
an existing remembered identity is removed when the user opts out.
In `@example-apps/dashnote/src/styles/globals.css`:
- Line 1: Replace the current `@import` rule that uses the url(...) form with the
bare string form to satisfy Stylelint's import-notation rule: change the line
'@import url("https://fonts.googleapis.com/...");' to use the plain string
syntax '@import "https://fonts.googleapis.com/...";' keeping the exact same font
URL and query parameters; update the import at the top of the stylesheet where
the Google Fonts import appears.
In `@example-apps/dashnote/test/App.test.tsx`:
- Around line 34-54: The mock for AppShell references React.ReactNode but React
is not imported, causing a TypeScript error; fix by importing the type and using
ReactNode instead: add an import for ReactNode from "react" at the top of the
test file and change the children prop type in the AppShell mock signature from
React.ReactNode to ReactNode (the other props can remain unchanged) so the type
reference resolves without importing the whole React namespace.
---
Nitpick comments:
In `@example-apps/dashnote/package.json`:
- Around line 20-24: The package.json currently lists "tailwindcss" and
"@tailwindcss/vite" under dependencies; move these two entries into
devDependencies so they are only installed for build/dev time. Edit package.json
to remove the "tailwindcss" and "@tailwindcss/vite" keys from the dependencies
object and add them with the same version strings under devDependencies, leaving
runtime deps like "react" and "react-dom" unchanged.
In `@example-apps/dashnote/src/components/NavButton.tsx`:
- Around line 22-28: The glyph <span> inside the NavButton component is being
read by screen readers alongside the label, causing redundant announcements;
update the glyph span (the element rendering {glyph} in NavButton.tsx) to
include aria-hidden="true" so the decorative glyph is ignored by assistive tech
while keeping the visible label accessible.
In `@example-apps/dashnote/src/components/NoteList.tsx`:
- Around line 61-81: The spinner SVG is duplicated in NoteList.tsx; extract it
into a reusable Spinner component (e.g., function Spinner({className}:
{className?: string}) or a React.FC<{className?: string}>) that renders the SVG
(preserve viewBox, aria-hidden, circle and path elements) and accepts a
className prop for the differing sizes/animations, then replace both inline SVGs
in the NoteList component with <Spinner className="..."/> (or the equivalent
JSX) and export/move Spinner to a shared components location if desired.
In `@example-apps/dashnote/src/styles/globals.css`:
- Line 4: Stylelint is flagging Tailwind v4 at-rules like
`@theme/`@source/@utility as unknown; update the stylelint configuration to allow
these by adding them to the scss/at-rule-no-unknown ignore list (add "theme",
"source", "utility", and "layer" to ignoreAtRules for the rule
"scss/at-rule-no-unknown") so Tailwind v4 CSS-first directives are accepted;
modify your project stylelint config (e.g., .stylelintrc) to include that
ignoreAtRules array.
In `@example-apps/dashnote/test/dash.test.ts`:
- Around line 88-123: Add a new test in the updateNote describe block that
covers the "note not found" branch by mocking sdk.documents.get to resolve to a
falsy value (e.g., null), calling updateNote with the same minimal args used in
existing tests, and asserting that the call rejects/throws the exact Error
message produced by updateNote (the "Note … not found." message); also ensure
mockDocumentConstructor is reset and sdk.documents.replace is not called in this
test so you validate the early exit path.
In `@example-apps/dashnote/test/queries.test.ts`:
- Around line 16-46: Add a new test case in the normalizeNotes describe block
that exercises the plain-object input branch: call normalizeNotes with a plain
object (e.g., { "note-1": { $ownerId: "...", $createdAt: "1000", $updatedAt:
2000n, $revision: "4", title: "...", message: "..." } }) and assert it returns
the same normalized array shape as the existing Map test (id, ownerId, createdAt
as number, updatedAt as number, revision as number, title, message); this
ensures the plain-object branch in queries.ts (the else path) is covered and
handles types like bigint and string revisions correctly.
In `@example-apps/dashnote/test/SessionContext.test.tsx`:
- Around line 243-245: The failing tests call act synchronously for handlers
like ref.current.forgetIdentity() and ref.current.logout(), which can miss async
state updates; update each unwaited act call (the blocks invoking
ref.current.forgetIdentity(), ref.current.logout(), and other ref.current.*
handlers at the highlighted locations) to be awaited—i.e., change act(() => {
... }) to await act(async () => { ... }) for every occurrence (lines flagged in
the review: the calls around forgetIdentity, logout, and the other referenced
blocks) so any microtasks or Promise-based state changes are flushed before
asserting.
- Around line 26-34: The tests currently rely on the mock SDK client for DPNS
resolution but do not intercept the ../src/dash/resolveDpnsName module; add an
explicit vi.mock for "../src/dash/resolveDpnsName" that returns a mocked
resolveDpnsName implementation (e.g., returning mockDpnsUsername or resolving
the same value the client.dpns.username stub provides) so SessionContext tests
only exercise the session layer; update the mock to replicate any expected
normalization/caching behavior used by SessionContext and ensure the mock is
imported/used alongside the existing mockRefreshContractCache mock.
- Line 49: Replace the hardcoded REMEMBERED_KEY string in the test with the real
constant exported from the production module: stop defining const REMEMBERED_KEY
= "dashnote.lastIdentity" and instead import the exported constant (e.g.,
REMEMBERED_KEY or REMEMBERED_IDENTITY_KEY) from rememberedIdentity.ts (or a
shared constants module) so tests follow the single source of truth; update the
test import and remove the local const to reference that exported symbol in
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2448eed1-bc90-4a02-82d8-49fad20deb7d
⛔ Files ignored due to path filters (1)
example-apps/dashnote/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (57)
example-apps/dashnote/.gitignoreexample-apps/dashnote/.prettierignoreexample-apps/dashnote/.prettierrc.jsonexample-apps/dashnote/CLAUDE.mdexample-apps/dashnote/README.mdexample-apps/dashnote/eslint.config.jsexample-apps/dashnote/index.htmlexample-apps/dashnote/package.jsonexample-apps/dashnote/public/dashnote-lite.htmlexample-apps/dashnote/src/App.tsxexample-apps/dashnote/src/components/AppShell.tsxexample-apps/dashnote/src/components/HowItWorks.tsxexample-apps/dashnote/src/components/IdentityCard.tsxexample-apps/dashnote/src/components/LoginModal.tsxexample-apps/dashnote/src/components/Modal.tsxexample-apps/dashnote/src/components/NavButton.tsxexample-apps/dashnote/src/components/NoteEditor.tsxexample-apps/dashnote/src/components/NoteList.tsxexample-apps/dashnote/src/components/NotesWorkspace.tsxexample-apps/dashnote/src/components/OperationResultNotice.tsxexample-apps/dashnote/src/components/Tabs.tsxexample-apps/dashnote/src/dash/client.tsexample-apps/dashnote/src/dash/contract.tsexample-apps/dashnote/src/dash/createNote.tsexample-apps/dashnote/src/dash/deleteNote.tsexample-apps/dashnote/src/dash/keyManager.tsexample-apps/dashnote/src/dash/queries.tsexample-apps/dashnote/src/dash/resolveDpnsName.tsexample-apps/dashnote/src/dash/types.tsexample-apps/dashnote/src/dash/updateNote.tsexample-apps/dashnote/src/hooks/useMediaQuery.tsexample-apps/dashnote/src/hooks/useTheme.tsexample-apps/dashnote/src/lib/fieldLimits.tsexample-apps/dashnote/src/lib/format.tsexample-apps/dashnote/src/lib/logger.tsexample-apps/dashnote/src/lib/notesCache.tsexample-apps/dashnote/src/lib/rememberedIdentity.tsexample-apps/dashnote/src/main.tsxexample-apps/dashnote/src/session/SessionContext.tsxexample-apps/dashnote/src/session/useSession.tsexample-apps/dashnote/src/styles/globals.cssexample-apps/dashnote/test/App.test.tsxexample-apps/dashnote/test/IdentityCard.test.tsxexample-apps/dashnote/test/LoginModal.test.tsxexample-apps/dashnote/test/NotesWorkspace.test.tsxexample-apps/dashnote/test/SessionContext.test.tsxexample-apps/dashnote/test/contract.test.tsexample-apps/dashnote/test/dash.test.tsexample-apps/dashnote/test/format.test.tsexample-apps/dashnote/test/notesCache.test.tsexample-apps/dashnote/test/queries.test.tsexample-apps/dashnote/test/rememberedIdentity.test.tsexample-apps/dashnote/test/resolveDpnsName.test.tsexample-apps/dashnote/tsconfig.app.jsonexample-apps/dashnote/tsconfig.jsonexample-apps/dashnote/tsconfig.node.jsonexample-apps/dashnote/vite.config.ts
Address actionable findings from CodeRabbit review on PR #74. Net effect: nine bug fixes / a11y improvements in dashnote, plus tighter test coverage for the cache, session, and reload-stale-response invariants. - a11y: drawer toggle aria-label flips with state; OperationResultNotice uses assertive aria-live for errors so role="alert" semantics hold - LoginModal: clear stale error and mnemonic when modal reopens - NotesWorkspace: monotonic reload token + session-snapshot guard so a late listMyNotes response from a previous identity/contract can't clobber state or write the wrong workspace into cache - notesCache: key by identityId + contractId + network (per CLAUDE.md); clearCachedNotes sweeps every contract+network slot for an identity - useTheme: optional-chain .matches so missing matchMedia falls back to "dark" instead of throwing - SessionContext: isolate DPNS lookup failures so a name-service hiccup doesn't fail an otherwise-valid session; clear any prior remembered identity when login uses rememberMe: false (prevents logout falling back to the wrong identity) - App.test.tsx: import ReactNode instead of using bare React.ReactNode (jsx: react-jsx doesn't put React in scope as a namespace) Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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.
Overview
Adds dashnote, a new Vite + React + TypeScript example app under
example-apps/that demonstrates note-taking on Dash Platform — identity login, DPNS resolution, and document CRUD against a notes data contract. It ships with a single-file read-only "lite" companion for quick demos.Highlights
public/dashnote-lite.htmlread-only companionCLAUDE.md, README, andsrc/layoutSummary by CodeRabbit
New Features
Documentation