refactor(dashnote): redesign workspace with toolbar, activity log, and sign-in hero#83
Open
thephez wants to merge 13 commits into
Open
refactor(dashnote): redesign workspace with toolbar, activity log, and sign-in hero#83thephez wants to merge 13 commits into
thephez wants to merge 13 commits into
Conversation
… log Implements P1–P3 from the design-review implementation guide: slim NotesToolbar replaces the heavy header card on Notes, editor toolbar surfaces rev/Updated/View JSON with JSON drawer and ⌘S Save, and a new ActivityPanel (⌘L) buffers structured log entries from the dash/* helpers so users can see SDK calls as they happen. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the padlock empty state on the Notes tab with a two-column teaching hero. Left column has the original "stores notes against your testnet identity" copy joined to the tutorial framing, a Sign in CTA, and a View source link to the dashnote folder on GitHub. Right column mirrors the real NoteEditor layout — Revision pill + "Updated", title + body, and the \$createdAt / \$updatedAt mono strip — so the preview matches what a logged-in user actually sees. The "no contract" branch still uses the original EmptyState. Also moves e.preventDefault() ahead of the bail-out gates in the NoteEditor ⌘S handler so the browser Save Page dialog never opens when the in-app save is unavailable (read-only, saving in flight, no changes, or oversize). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the double timestamp on each row to a single relative time, gives the preview the full row width by anchoring the timestamp to the title line, and italicizes the fallback when a note has no title so untitled rows read as derived rather than authored. Adds a global "/" hotkey that focuses the list search input (skipped when typing in any text field) and a "/" hint chip inside the search input. Selected rows now show a thin accent bar flush to the left edge of a tinted lg-rounded card instead of a full accent outline; the bar is absolutely positioned so the row text does not shift on select. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Widens Modal's title prop to accept a ReactNode so the login screen can replace the small uppercase eyebrow with a 28-px key glyph, "Sign in to Dashnote" title, and a "Connects to testnet" subtitle. The privacy reassurance moves directly under the password input as a shield-glyphed "Stored in memory only — never sent over the network" line, replacing the trailing paragraph. The Bridge prompt becomes a dedicated dashed-bordered card at the bottom of the form, gated on the no-remembered-identity path so the "hides when remembered" test still holds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P5 replaced the old "Personal notes on Dash Platform" EmptyState heading with the SignInHero's "Personal notes, stored on a public blockchain." The tab-navigation smoke spec was still asserting the old copy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the small "Sign in to edit" toolbar button with a transparent overlay layered above the disabled inputs so any click on the editor body opens the login modal. The disabled inputs stay disabled underneath, so a missed click can't mutate state. Read-only viewing still looks identical to before — same dimmed text, same cursor on the fields themselves; only the surface gains a click target. Adds the same sign-in affordance to the note list create paths so a remembered-but-signed-out user can act: the desktop "New note" pill becomes "Sign in to create" and the mobile compose "+" is now visible in browsing mode; both call onOpenLogin instead of starting a draft. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the two-paragraph stub with a four-section tutorial map: - Data flow: product-model intro plus a four-card pipeline (UI → Helper → Evo SDK → Platform). Cards are progressively accent-tinted to reinforce direction, and each links to a canonical target (GitHub source for UI/Helper, npm for Evo SDK, Platform Explorer testnet for Platform). - Platform operations: five-row table where each row links to a one-file helper in src/dash/, with the SDK call name alongside. - Inline code peek of the simplest mutation (createNote.ts), with a View full file link. - Recommended source files: numbered-pill reading order, each row a clickable link to its GitHub source. - Continue to docs CTA: solid surface, accent-tinted icon background, hover fill — reads as a primary action rather than a disabled tag. Also drops the page-header eyebrow on App.tsx so the section labels inside HowItWorks are the canonical typographic anchors. Adds HowItWorks.test.tsx covering pipeline labels, ops-table links, the code peek, the reading-order list, and the new link behavior on the pipeline cards and reading-order rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the App-shell page-header card on the how-it-works tab so the HowItWorks component owns its own header instead of stacking under one. The new in-page header lifts the previous data-flow narrative paragraph into the lead, and the revision/timestamps follow-up moves into the Platform-operations intro where it's contextually relevant. The data-flow card is now a pure diagram: pipeline cards lose the left-to-right accent gradient in favor of a flat bg, and each card leads with an accent-colored glyph (grid for UI, code-brackets for Helper, link for Evo SDK, shield for Platform) so the four steps read as distinct affordances rather than tinted siblings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the connection-dot indicator and relabel the IdentityCard states by identity status: Guest / Remembered / Signed in, with Full access and Read-only access subtitles. Adds a document glyph to the sidebar logo and resizes the identity avatar. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Strip the splash hero's border and rounded corners on mobile so it goes edge-to-edge, and center its text and buttons vertically and horizontally. Update e2e specs and the Playwright fixture to match the new IdentityCard labels (Guest / Connected / Read-only access / Full access) — the prior `.conn-dot.connected` boot gate is gone, so each wait now anchors on the corresponding subtitle text instead. Also fix the How it works heading match against the new walkthrough headline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Treat a remembered identity as "signed out with hint" rather than its own first-class state — the IdentityCard now shows "Signed out" / "Read-only access" with the cached DPNS row, and clicking the card opens the login modal directly (no menu). The authenticated branch keeps its menu (Settings / Switch identity / Log out). Updates unit tests to assert the eyebrow + subtitle pair per state, and reworks the Playwright fixtures so loginViaModal clicks the IdentityCard itself in browsing mode. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… helpers Revert the structured log payloads added in 8468756 from createNote / updateNote / deleteNote so each helper is "just the SDK call" again. NotesWorkspace now wraps the session logger via withDetail() to attach the activity-panel detail string and emits the success row itself. updateNote returns the new revision so the caller can render `rev N` without recomputing +1n. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds structured activity logging with UI panel and hotkey, JSON preview and conflict warnings in the note editor, keyboard/search improvements in the note list, identity/login UI updates, a HowItWorks walkthrough rewrite, removal of connection-dot styling, and corresponding unit/e2e test updates. ChangesActivity Logging & App Integration
Note Editor & List Display Enhancements
Identity, Auth, HowItWorks UI Redesigns & E2E Test Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
example-apps/dashnote/test/DeleteNoteModal.test.tsx (1)
92-102: ⚡ Quick winPrefer
.toBeDisabled()matcher for clearer assertions.Testing Library provides a
.toBeDisabled()matcher that is more semantic and idiomatic than checking.hasAttribute("disabled"). It also produces clearer error messages when assertions fail.✨ Suggested refactor
expect( - screen - .getByRole("button", { name: /deleting…/i }) - .hasAttribute("disabled"), - ).toBe(true); + screen.getByRole("button", { name: /deleting…/i }), + ).toBeDisabled(); expect( - screen - .getByRole("button", { name: /^cancel$/i }) - .hasAttribute("disabled"), - ).toBe(true); + screen.getByRole("button", { name: /^cancel$/i }), + ).toBeDisabled();🤖 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/DeleteNoteModal.test.tsx` around lines 92 - 102, Replace the manual attribute checks in the DeleteNoteModal test with the semantic matcher: instead of asserting screen.getByRole("button", { name: /deleting…/i }).hasAttribute("disabled") and the same for screen.getByRole("button", { name: /^cancel$/i }), use the Testing Library matcher .toBeDisabled() on those getByRole results (i.e., expect(screen.getByRole("button", { name: /deleting…/i })).toBeDisabled() and expect(screen.getByRole("button", { name: /^cancel$/i })).toBeDisabled()) so the assertions are clearer and produce better failure messages.example-apps/dashnote/src/components/ActivityPanel.tsx (1)
26-29: ⚡ Quick winMark the panel as a modal dialog for assistive tech.
On Line 27, add
aria-modal="true"to therole="dialog"container so screen readers treat background content as non-interactive while the panel is open.Proposed patch
<div role="dialog" + aria-modal="true" aria-label="Activity log" className="fixed inset-0 z-40 flex justify-end bg-black/40" onClick={onClose} >🤖 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/ActivityPanel.tsx` around lines 26 - 29, Add the missing aria-modal attribute to the dialog container in the ActivityPanel component so assistive tech treats background content as non-interactive; update the element that currently has role="dialog" (the top-level div in ActivityPanel) to include aria-modal="true" alongside role="dialog" and aria-label="Activity log".example-apps/dashnote/test/App.test.tsx (1)
169-184: ⚡ Quick winAdd a Ctrl+L assertion for parity with runtime behavior.
The test covers
metaKeyonly; addingctrlKeyverifies the non-mac path already implemented inApp.Proposed patch
it("opens the activity panel via ⌘L hotkey and toolbar button", () => { @@ // Toggling again closes it. fireEvent.keyDown(window, { key: "l", metaKey: true }); expect(screen.getByText("activity:false")).toBeTruthy(); + + fireEvent.keyDown(window, { key: "l", ctrlKey: true }); + expect(screen.getByText("activity:true")).toBeTruthy(); fireEvent.click(screen.getByRole("button", { name: /open activity/i })); expect(screen.getByText("activity:true")).toBeTruthy(); });🤖 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/App.test.tsx` around lines 169 - 184, The test currently only simulates the macOS hotkey using fireEvent.keyDown(window, { key: "l", metaKey: true }) in the "opens the activity panel via ⌘L hotkey and toolbar button" test; add an equivalent assertion that sends the non-mac hotkey (fireEvent.keyDown with ctrlKey: true) to verify App toggles activity the same way. Locate the test in example-apps/dashnote/test/App.test.tsx and insert a fireEvent.keyDown(window, { key: "l", ctrlKey: true }) call followed by the same expect(...) check used after the metaKey events to assert activity toggles.example-apps/dashnote/src/dash/updateNote.ts (1)
1-8: ⚡ Quick winUpdate JSDoc to document the return value.
The function signature now returns
Promise<bigint>(the saved note's revision), but the JSDoc block does not document this return value.📝 Suggested JSDoc enhancement
/** * Update an existing note. Fetches the current document to bump its revision, * then submits a replace state transition. * * SDK methods: * sdk.documents.get(contractId, documentTypeName, documentId) * sdk.documents.replace({ document, identityKey, signer }) + * + * `@returns` Promise resolving to the new revision number after successful save */🤖 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/dash/updateNote.ts` around lines 1 - 8, The JSDoc for updateNote is missing documentation of its return value; update the comment for the updateNote function to include an `@returns` (or `@return`) entry describing that the function returns Promise<bigint> representing the saved note's revision (the bumped document revision after replace), and briefly note any failure behavior (e.g., it throws on error). Keep the description concise and match the return type exactly: Promise<bigint>.
🤖 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/NotesToolbar.tsx`:
- Around line 49-50: The displayed shortcut hint in NotesToolbar (the span
rendering "⌘L") is misleading because the app supports both Command and Control;
update the label in the span inside NotesToolbar (the rounded border span
element) to indicate both modifiers (e.g., "⌘/Ctrl L" or "Ctrl/Cmd L") and also
update any aria-label/title text for the same element so keyboard users on
Windows/Linux see the correct shortcut; ensure the visible text and any
accessible label match the actual handler that listens for Cmd and Ctrl.
In `@example-apps/dashnote/test/e2e/notes.spec.ts`:
- Around line 244-246: The selector for "Read-only access" is not scoped to the
sidebar; update the assertion to scope the lookup to the sidebar locator used
elsewhere by calling the sidebar locator (aside[aria-label="Main navigation"])
and then using getByText("Read-only access", { exact: true }) on that locator so
the check (toBeVisible with the existing { timeout: 60_000 }) targets the main
navigation only.
- Around line 219-221: The assertion uses page2.getByText("Read-only access")
without scoping to the main navigation sidebar; change it to scope the lookup to
the sidebar locator (e.g., replace page2.getByText("Read-only access", { exact:
true }) with something like
page2.locator('<sidebar-selector>').getByText("Read-only access", { exact: true
}) or use the same sidebar helper used elsewhere so the check is limited to the
sidebar (update the expression that calls page2.getByText on "Read-only access"
to be invoked on the sidebar locator instead).
---
Nitpick comments:
In `@example-apps/dashnote/src/components/ActivityPanel.tsx`:
- Around line 26-29: Add the missing aria-modal attribute to the dialog
container in the ActivityPanel component so assistive tech treats background
content as non-interactive; update the element that currently has role="dialog"
(the top-level div in ActivityPanel) to include aria-modal="true" alongside
role="dialog" and aria-label="Activity log".
In `@example-apps/dashnote/src/dash/updateNote.ts`:
- Around line 1-8: The JSDoc for updateNote is missing documentation of its
return value; update the comment for the updateNote function to include an
`@returns` (or `@return`) entry describing that the function returns Promise<bigint>
representing the saved note's revision (the bumped document revision after
replace), and briefly note any failure behavior (e.g., it throws on error). Keep
the description concise and match the return type exactly: Promise<bigint>.
In `@example-apps/dashnote/test/App.test.tsx`:
- Around line 169-184: The test currently only simulates the macOS hotkey using
fireEvent.keyDown(window, { key: "l", metaKey: true }) in the "opens the
activity panel via ⌘L hotkey and toolbar button" test; add an equivalent
assertion that sends the non-mac hotkey (fireEvent.keyDown with ctrlKey: true)
to verify App toggles activity the same way. Locate the test in
example-apps/dashnote/test/App.test.tsx and insert a fireEvent.keyDown(window, {
key: "l", ctrlKey: true }) call followed by the same expect(...) check used
after the metaKey events to assert activity toggles.
In `@example-apps/dashnote/test/DeleteNoteModal.test.tsx`:
- Around line 92-102: Replace the manual attribute checks in the DeleteNoteModal
test with the semantic matcher: instead of asserting screen.getByRole("button",
{ name: /deleting…/i }).hasAttribute("disabled") and the same for
screen.getByRole("button", { name: /^cancel$/i }), use the Testing Library
matcher .toBeDisabled() on those getByRole results (i.e.,
expect(screen.getByRole("button", { name: /deleting…/i })).toBeDisabled() and
expect(screen.getByRole("button", { name: /^cancel$/i })).toBeDisabled()) so the
assertions are clearer and produce better failure messages.
🪄 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: 5e3a2e75-874b-4e90-8d6b-e353cc5f885f
📒 Files selected for processing (29)
example-apps/dashnote/src/App.tsxexample-apps/dashnote/src/components/ActivityPanel.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/NoteEditor.tsxexample-apps/dashnote/src/components/NoteJsonDrawer.tsxexample-apps/dashnote/src/components/NoteList.tsxexample-apps/dashnote/src/components/NotesToolbar.tsxexample-apps/dashnote/src/components/NotesWorkspace.tsxexample-apps/dashnote/src/dash/updateNote.tsexample-apps/dashnote/src/lib/logger.tsexample-apps/dashnote/src/session/SessionContext.tsxexample-apps/dashnote/src/styles/globals.cssexample-apps/dashnote/test/ActivityPanel.test.tsxexample-apps/dashnote/test/App.test.tsxexample-apps/dashnote/test/DeleteNoteModal.test.tsxexample-apps/dashnote/test/HowItWorks.test.tsxexample-apps/dashnote/test/IdentityCard.test.tsxexample-apps/dashnote/test/NoteEditor.test.tsxexample-apps/dashnote/test/NoteList.test.tsxexample-apps/dashnote/test/NotesWorkspace.test.tsxexample-apps/dashnote/test/e2e/auth.spec.tsexample-apps/dashnote/test/e2e/fixtures.tsexample-apps/dashnote/test/e2e/notes.spec.tsexample-apps/dashnote/test/e2e/settings.spec.tsexample-apps/dashnote/test/e2e/smoke.spec.ts
💤 Files with no reviewable changes (1)
- example-apps/dashnote/src/styles/globals.css
— cross-platform shortcut hint, modal a11y, scoped e2e selectors
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
src/dash/*helpers so dash helpers stay UI-agnostic.Notable changes
ActivityPanel,NotesToolbar,NoteJsonDrawerHowItWorks,NoteEditor,NotesWorkspace,LoginModal,IdentityCardActivityPanel,App,HowItWorks,NoteEditor,NoteList; updated e2e specs (auth,notes,settings,smoke) and fixtures to match the new UITest plan
npm run buildandnpm run testinsideexample-apps/dashnoteSummary by CodeRabbit
New Features
Improvements
Tests