feat(notes): add workspace note tabs - #187
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 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 |
Deploying everfreenote with
|
| Latest commit: |
b2469ef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://32a966fb.everfreenote.pages.dev |
| Branch Preview URL: | https://codex-notes-tabs.everfreenote.pages.dev |
Contributor
PR StatusUpdated for PR #187 at 📊 Allure Test ReportContributing Workflows
Catalog: All reports 🤖 Android Build PanelCheck a box below to trigger a release build:
|
Contributor
Contributor
Qodana for JS317 new problems were found
@@ Code coverage @@
+ 91% total lines covered
12601 lines analyzed, 11569 lines covered
+ 91% fresh lines covered
933 lines analyzed, 858 lines covered
# Calculated according to the filters of your coverage tool💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
Owner
Author
|
@codex review please |
|
To use Codex here, create a Codex account and connect to github. |
Owner
Author
|
@codex review please |
|
To use Codex here, create a Codex account and connect to github. |
Owner
Author
|
@codex review please |
|
To use Codex here, create a Codex account and connect to github. |
koreyba
marked this pull request as ready for review
August 2, 2026 08:02
Many tabs blew the page width: the NotesShell column, row, main, and editor-pane flex items were missing min-w-0, so the strip's intrinsic width propagated up and pushed the whole layout (NavRail off-screen on desktop, tab controls off-screen on mobile). Tabs now scroll inside the strip viewport as designed; the mobile tab header stays constrained. Also reset the active tab by id after a delete: the save handler clears the tab's noteId before the workspace reset runs, so the noteId-based reset alone left a stale draft as the tab label. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…functionality-8ee53c
…at the mobile menu Browser pass over the notes workspace tabs at 320-1440px with 1, 2, 8 and 24 tabs found the tab strip treating "runs out of horizontal room" as a capacity limit, and the mobile tab list displacing the note it should float over. Desktop strip: - Add is gated on MAX_NOTE_WORKSPACE_TABS only. The measured width previously capped the workspace at 8 tabs on a 1440px window and 4 on a 1024px one, and announced "limit reached: 4 tabs" while eight tabs were open. - Chevron controls and wheel-to-horizontal scrolling reach the overflow; macOS overlay scrollbars stay invisible until a scroll is already running. - The native scrollbar is hidden, so the strip no longer grows 43px -> 54px when it starts overflowing. - A ResizeObserver re-reveals the active tab, which otherwise scrolled out of sight on resize, and scrollIntoView now targets the whole tab so its close button is inside the viewport too. - Scroll snapping stops the strip from leaving a tab clipped down to just its close button. Mobile menu: - Absolutely positioned popover; in flow it pushed the note down 545px. - Dismisses on outside pointer-down and on Escape. - Scrolls the active row into view on open. Also fall back to the live draft title in the close-tab confirmation, which said "this tab" for an unsaved new note. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The editing header clipped its "more actions" control off the right edge of a
375px screen. The buttons were not the cause: the editor column is a flex item
without min-w-0, so it could not shrink below the intrinsic width of the
formatting toolbar and grew to 397px, carrying the absolutely positioned
header past the viewport with it.
- min-w-0 on both note panes, so the column tracks the pane width. The header
now ends 12px inside the screen at 320px, the narrowest size we support,
which also clears a pre-existing clip at that width.
- A shrinkable mode label plus a shrink-0 action group, so the row cannot push
a control out of view again if the header gains content.
- Read/Save/Edit/Delete collapse to icons below md, keeping their labels as
aria-label. Desktop keeps the text.
- The mode label ("Reading"/"Editing") is screen-reader only on phones. The
tab bar directly above names the note and the actions name the mode.
- The back control matches the other pill buttons, and gained the aria-label
it was missing in reading mode.
Two follow-on fixes from that work:
- The sticky formatting toolbar parked at a hardcoded 69px, which no longer
matched the header and left an 8px strip of scrolling text between them.
NoteEditor now publishes the measured header height as
--note-editor-header-h and the toolbar follows it.
- Reading and editing hid the native scrollbar differently, so only one of the
two showed a stray line beside the action bar. Both use scrollbar-none now.
NoteEditor.cy.tsx targets Save/Read by aria-label instead of button text,
which no longer resolves at every viewport.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signing in as a second user in the same browser tab restored the first user's tabs: the strip listed their note titles and the cached note bodies were readable straight out of sessionStorage. Workspace state carried no account stamp, and clearNoteWorkspaceState() was never called from app code. Two layers close it: - NoteWorkspaceState carries a userId. hydrateNoteWorkspaceState compares it against the account being restored for and starts a fresh workspace on any mismatch, treating state with no stamp as foreign. This also covers the paths that never reach a clean sign-out: a crash, an expired token, a session replaced some other way. - handleSignOut clears the stored workspace, so the cached notes are gone at sign-out rather than waiting for the next sign-in to discard them. The hook re-hydrates when the user id changes and only writes once its state belongs to the account currently signed in, so the outgoing account's tabs are never rewritten under the incoming one. readNoteWorkspaceState now takes an options object; the positional (storage, idFactory) form could not gain a userId without reading ambiguously at the call sites. Verified against two seeded users on a local stack: sign-out empties the store, the second sign-in gets its own stamp and a single blank tab, and a foreign-stamped blob planted directly into sessionStorage is discarded instead of rendered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The account tab could only delete the account — the one way out of a session was the sidebar icon, which the mobile layout hides. Sign out now sits between the email and the destructive block, so the reversible action comes first. Full width on phones, inline beside its description from sm up. It reuses the existing sign-out flow, so it clears the workspace tabs and returns to the sign-in screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The note action bar and formatting toolbar read as unpolished on phones. Compared side by side at the sizes the app actually renders, Phosphor regular is the softer, more even set in a dense toolbar. The swap is app-wide, including the vendored components/ui primitives: the Select caret and Checkbox tick sit directly beside note-toolbar icons, so a partial swap would show. 77 icon names were remapped; local identifiers are preserved with import aliases (TextB as Bold), so JSX bodies are untouched. - next.config.js lists the package under optimizePackageImports. It is a barrel of ~1500 icons and this app is a static export; verified against a production build that unreferenced icons stay out of the chunks. - Lucide emitted per-icon CSS classes and six Cypress selectors keyed on them (.lucide-chevron-left, svg.lucide-tag, .lucide-log-out, svg.lucide-sun). Those broke at once. Stable data-cy hooks were added to the note action bar, the tab strip, the mobile tab menu, the sidebar sign-out, the theme toggle and the tag input, and the specs now target those. - cypress/README.md records the rule: select on data-cy or data-testid, never on an icon library's classes and never on button text a breakpoint can hide. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closing a tab whose save failed threw the edits away behind window.confirm — a blocking native dialog in an app where every other destructive action uses AlertDialog, and one that browsers can suppress outright. The controller now surfaces the pending close as state (tabPendingClose) and NotesShell renders it, matching the delete-note dialog. The wording says what is lost rather than asking a bare yes/no, and the buttons name their outcome: "Keep editing" / "Discard and close". The confirmed path skips the autosave flush, which would only fail again on a tab that is in this state precisely because saving failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Component coverage for the dialog itself: it renders with the failed tab's name, "Keep editing" cancels without closing, and "Discard and close" reports the confirmation. The controller side (error state to pending close to confirm/cancel) is covered in useNoteAppControllerAdditional. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…udget
Each tab caches the note it shows, so the 2MB session budget was reachable:
at 60KB note bodies only 17 tabs fit while the UI allows 32. Past that the
write threw, writeNoteWorkspaceState swallowed it, and the whole workspace
silently stopped persisting — a reload lost every tab with no indication.
- A clean tab's draft is a byte-for-byte copy of its note, so it is no longer
written. Hydration already rebuilds it from the note, so the round trip is
unchanged. Per-tab cost halves: 60KB notes now fit 34 tabs, above the cap.
- Over budget, the snapshot keeps the active tab, then every tab carrying
unsaved work, then as many saved tabs as fit. A reload restores fewer tabs
instead of none. Losing a saved note costs a re-open; losing a draft costs
the user's typing.
- Dropping tabs is no longer silent: one warning per session, not per write,
since persistence runs on every workspace change.
writeNoteWorkspaceState now reports { persisted, droppedTabIds } instead of a
bare boolean, which is what lets the hook say something.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A tab disappearing made the strip jump, with the remaining tabs teleporting into the gap. A closed tab now collapses over 150ms while the rest settle: horizontally in the desktop strip, vertically in the mobile list, from one shared hook so the two cannot drift apart. Closing is not delayed by any of this — the tab leaves workspace state immediately. useAnimatedTabList only keeps a non-interactive copy (aria-hidden, inert, no pointer events) in the same slot until the exit ends. The exit is a CSS keyframe animation, not a transition: the copy is created already in its end state, so a transition would have nothing to run from. This was measured wrong twice before landing on it, once because requestAnimationFrame does not fire while the document is hidden. Opening deliberately does not mirror the collapse. Closing the gap is a layout requirement; a new tab arrives in space that already exists, and expanding it while the strip may also be scrolling to reveal it reads as busy. New tabs fade and scale in over the same duration instead. prefers-reduced-motion drops all of it: the hook skips the copy, and a new global rule neutralises animation and transition durations across the app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
New Tests in Allure Report Allure ReportShow 6 new tests
|
|
This branch was successfully deployed
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
2 participants
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
Validation
npm run type-checknpx eslint . --max-warnings=0npx ai-devkit@latest lint --feature notes-tabsScope notes
ui/mobileremains unchanged in this branch