Repository navigation
Worktree miui notes redesign - #2
Conversation
The app rendered as a flat black field on device for three reasons, all fixed here: - The dark palette had no internal contrast -- canvas #0d0c0b, paper #121110, card #1c1a17 and line #2a2723 are four indistinguishable near-blacks, so cards never separated from the background and borders were invisible. Replaced with an OLED-black canvas, a lifted card, and a single amber accent. - darkMode 'class' was only synced to NativeWind on web, so on device every `dark:` variant silently no-opped while useTheme() went dark anyway -- dark navigator chrome behind light-styled content. - SystemUI now paints the window behind the React root, so there is no white flash between the splash tearing down and the first frame. Also lays the groundwork for the reference feature set: - Migration v2 adds notebooks, notes.notebook_id, locked and pinned_at. A note with a null notebook_id is in the Default notebook; modelling the default as the absence of a row keeps it undeletable and leaves no per-install id to reconcile when sync lands. - @dailynote/core/formatting: pure string transforms for block styles, lists, checklists, indentation, renumbering, list continuation on Enter and inline marks. A TextInput renders one uniform text style, so the body stays plain text carrying Markdown-ish markers -- the same way the reference editor shows literal 1. / 2. / 3. as you type. 69 tests, since none of this is verifiable on a simulator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces the composer-as-launch-screen with the shape the reference uses: the library is the front door and capture is reached through the floating button. Screens: - (tabs)/index -- large title, note count, notebook filter rail, and a grouped card whose rows share one surface with hairline dividers. Date and preview share a line, as in the reference. Amber FAB. - (tabs)/todos -- a view over the notes table, not a second store: a to-do is a note that was promoted to a commitment. Due-ness ticks on a timer so an item moves into Due while you are looking at it. - notebooks -- the drawer: All notes, the user's notebooks with counts and swatches, then the fixed Default / Locked / Recently deleted shelves. One sheet serves both create and rename, because Alert.prompt is iOS-only and would have silently done nothing on Android. - note/[id] -- back/undo/redo/overflow/done, the meta line (edited-at, character count, notebook), a Heading field and body, and the editing rail. The title is the body's first line rather than its own column, which is what keeps deriveTitle honest: renaming a note and editing its first line are the same act. - trash, locked -- soft-delete recovery, and notes behind the device lock. Locking delegates to the OS rather than inventing an in-app PIN that would then have to be stored somewhere. Format sheet applies the core transforms and doubles as a readout of what the caret is already inside. Alignment is whole-note because a TextInput has one textAlign for all of its content. The Add sheet is present but inert, and says so: real attachments need a file store and an editor that can render them inline, and neither exists yet. Better to show the shape and be honest than to fail silently on tap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It opened the notebooks drawer while announcing itself as "More", which also made it indistinguishable from the editor's own overflow button to anything reading by label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 26 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe PR adds notebook organization, locked notes, trash management, to-do views, and a full rich-text note editor. It updates SQLite persistence, core note types, mobile navigation, theming, autosave, formatting, and device authentication. ChangesCore formatting and persistence
Mobile editor and navigation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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: 26
🤖 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 `@apps/mobile/src/app/`(tabs)/_layout.tsx:
- Around line 22-30: The tabBarStyle.height is hard-coded to 62 without
accounting for the bottom safe area inset, which can cause tab screens with
restricted edges to overlap the safe area. Use the useSafeAreaInsets() hook to
retrieve the bottom inset and add it to the hard-coded height value in the
tabBarStyle object, or alternatively remove the explicit height property and let
the navigator handle the layout.
In `@apps/mobile/src/app/`(tabs)/index.tsx:
- Around line 60-71: Update apps/mobile/src/app/(tabs)/index.tsx lines 60-71 in
the rootNotes useMemo predicate to also require !note.deleted and
note.archivedAt === null, preserving the existing kind and locked checks. Apply
the same visibility filtering in apps/mobile/src/app/(tabs)/todos.tsx lines
34-37 within commitments so deleted or archived to-dos are excluded from all
sections; prefer a shared isVisible(note) predicate if the existing structure
supports it.
In `@apps/mobile/src/app/`(tabs)/todos.tsx:
- Around line 109-117: The accessibilityLabel in the Pressable component with
hitSlop=10 is static and always announces "Mark <title> done", but the onPress
handler toggles the complete state in both directions. Update the
accessibilityLabel to be conditional based on the complete variable so that when
complete is true, it announces an action that marks the note incomplete (such as
"Mark <title> not done"), and when complete is false, it announces "Mark <title>
done".
In `@apps/mobile/src/app/locked.tsx`:
- Around line 36-47: Update the authenticate promise chain in the useEffect for
the locked screen to handle rejections by setting the gate to 'locked', while
still honoring the active guard to avoid state updates after cleanup. Keep the
existing success-result handling unchanged so the Unlock retry path remains
available when authenticate fails.
In `@apps/mobile/src/app/note/`[id].tsx:
- Around line 513-514: The destructive row currently uses theme.muted for the
tint color when destructive is true, which de-emphasizes the action instead of
signaling risk. Replace theme.muted with the danger token from the theme palette
in the tint assignment's ternary expression so destructive actions render with
the appropriate warning color.
- Around line 117-134: Update the persist callback to serialize overlapping
writes using a shared promise or equivalent write queue, ensuring each
invocation observes the latest noteId.current and completes before the next
begins. Move savedRef.current assignment until after create or setBody succeeds,
while preserving the empty-new-note guard and returning the awaited write
promise so done, unmount flushing, requireSaved, and delete/action flows wait
for persistence and retry after failures.
- Around line 100-112: Resolve a live note record from the store using
noteId.current after the existing noteId ref is initialized, and use that record
for current filing state. Update the metadata, overflow menu, and notebook sheet
reads of note?.pinnedAt and note?.notebookId to use live?.pinnedAt and
live?.notebookId, while preserving the existing behavior for other captured note
fields.
- Around line 92-93: Normalize the title in the title input’s change handler so
newline characters are removed or truncated before updating the title state.
Update the handler associated with title and setTitle, preserving the existing
composed serialization and body behavior while ensuring pasted multiline text
cannot move into the body on reopen.
In `@apps/mobile/src/app/notebooks.tsx`:
- Around line 39-58: Update the counts useMemos in the notebooks screen to skip
deleted entries before accumulating total, unfiled, locked, or byNotebook
values, and make trashCount include only non-deleted? no—only entries whose kind
is "note" while retaining deleted filtering. Ensure all counts match the
note-only, non-deleted list behavior and the trash list.
- Around line 229-246: Update the swatch Pressable accessibilityLabel in the
SWATCHES map to use a human-readable display name for each palette key instead
of exposing the raw key. Reuse or add the appropriate key-to-display-name
mapping near the color selection logic, while preserving the existing
accessibility state and press behavior.
In `@apps/mobile/src/app/trash.tsx`:
- Around line 98-101: Update the onPurge handler in the trash row to require
explicit confirmation before invoking purge(note.id). Reuse the existing
confirmation flow used by the bulk delete path, preserving the destructive
action only after confirmation and leaving cancellation without side effects.
- Around line 34-50: Align the empty-trash confirmation with the rows it
displays by updating the purge flow used by confirmEmpty to delete only
soft-deleted notes (kind === 'note'). Modify the repository method purgeAll in
notes-repo.ts and its caller as needed, preserving the existing confirmation
count and ensuring to-do rows remain untouched.
In `@apps/mobile/src/components/format-sheet.tsx`:
- Around line 46-60: Remove the unused required onEdit prop from
FormatSheetProps and the FormatSheet component, then remove the corresponding
Edit import. Update the note screen call site to stop passing onEdit={apply},
while preserving all existing formatting callbacks.
In `@apps/mobile/src/components/sheet.tsx`:
- Around line 38-45: Update the outer Pressable scrim in the sheet component to
provide an accessibility label, and mark the inner Pressable responder wrapper
around the Animated.View as inaccessible so it does not group or hide the
sheet’s individual controls from screen readers.
In `@apps/mobile/src/db/migrations.ts`:
- Around line 61-88: Update the v2 migration callback in migrations.ts to
backfill every existing notes.title value after the schema changes: read the
existing note rows, derive each title in JavaScript using deriveTitle (or
equivalent frozen logic), and persist the recomputed title through the migration
database handle. Keep the backfill within v2 and ensure it runs for all
pre-existing notes before the migration completes.
In `@apps/mobile/src/db/notes-repo.ts`:
- Around line 133-142: Measure the `all()` query performance under larger note
counts before changing it. If indexing is warranted, add a migration creating a
partial index on non-deleted notes over `pinned_at` and `updated_at`, and verify
it supports the ordering used by `all()` without changing the query’s
pinned-first behavior.
- Around line 206-213: Update the purge method's SQL delete statement to scope
the hard delete to soft-deleted rows only. Modify the condition in the delete
query to include both the id check and the deleted = 1 condition, matching the
scoping pattern already implemented in the purgeAll method. This ensures purge
only hard-deletes rows that were already soft-deleted, aligning the
implementation with its documented behavior.
In `@apps/mobile/src/lib/format.ts`:
- Around line 41-53: Duplicate date formatting functions exist locally in
multiple files, preventing the new centralized implementations in format.ts from
being used. Remove the local shortDate implementations from
apps/mobile/src/app/(tabs)/index.tsx and apps/mobile/src/app/trash.tsx, remove
the local editedAt implementation from apps/mobile/src/app/note/[id].tsx, and
update those three files to import the shortDate and editedAt helpers from the
format module instead. This ensures a single source of truth for date formatting
behavior across the application.
In `@apps/mobile/src/lib/lock.ts`:
- Around line 4-16: Document in the LockResult/locking documentation and
lock-note UI copy that locking hides notes but does not protect plaintext bodies
from file-level access. Verify the main notes list excludes locked rows before
rendering, preserving the filter around the all() repository results; also check
other note-rendering views for the same requirement. If confidentiality is
intended, encrypt locked note bodies with a key stored via SecureStore and only
retrieve it after authenticate, while preserving existing note behavior.
- Around line 24-36: Update authenticate to catch rejections from
LocalAuthentication.authenticateAsync and convert them into a failure
LockResult, ensuring every execution path fulfills Promise<LockResult> so
locked.tsx can recover. Apply the same rejection handling to canLock around its
isEnrolledAsync call if that API can reject on supported platforms.
In `@apps/mobile/src/lib/undo.ts`:
- Around line 45-63: The coalescing logic in the set function allows unlimited
continuation of a typing run as long as keystrokes occur within the 700ms
window, and the past array grows unbounded with full text snapshots. Introduce a
new ref to track when the current coalescing run started (distinct from
lastEntryAt.current which updates on each keystroke), and modify the coalesce
condition to also check whether the run duration has exceeded a reasonable upper
bound interval, closing the entry when that bound is crossed. Additionally, cap
the length of past.current by removing oldest entries once it reaches a maximum
size, ensuring history depth remains bounded while preserving the most recent
snapshots.
In `@apps/mobile/src/store/notebooks-store.tsx`:
- Around line 47-60: The initial load in the notebooks store lacks rejection
handling. Add a catch handler to the repo.all() promise chain in the useEffect,
logging the failure while preserving the active guard and loading cleanup; apply
the same fix to the analogous initial load in notes-store.tsx.
- Around line 95-98: Update the NotebooksProvider remove method to own notes
invalidation after repo.remove(id), using the chosen refresh-callback or
provider-nesting approach so both notebooks and notes refresh before returning.
Then remove the redundant refreshNotes workaround from the notebooks screen and
preserve the existing provider contract consistently for all callers.
In `@apps/mobile/src/theme/colors.json`:
- Around line 9-12: Update the faint and accentInk color values in the theme
palette to meet WCAG AA contrast: darken faint.light against canvas.light,
lighten faint.dark against canvas.dark, and adjust accentInk values if needed
for readable text on accent backgrounds. Verify all affected palette pairs with
the provided contrast-check script, preserving the existing light/dark theme
structure.
In `@packages/core/src/formatting.test.ts`:
- Around line 166-169: Rename the existing test to describe applying bullet
styling to a checklist and dropping its ticks. Add a separate test covering
reapplying checklist styling to an existing checklist, asserting the intended
toggle-to-none behavior reported by listStyleAt and the resulting unchecked/list
output.
In `@packages/core/src/formatting.ts`:
- Around line 357-409: Update hasInlineMark and toggleInlineMark to exclude
doubled '*' runs when mark is italic, so bold text is not detected as italic and
toggling italic wraps rather than strips bold markers. Reuse a shared helper for
both functions, and add coverage for hasInlineMark on **bold** returning false
and toggleInlineMark producing the intended nested italic result.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 982b71fa-d777-4959-baba-21faaa353310
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (31)
apps/mobile/package.jsonapps/mobile/src/app/(tabs)/_layout.tsxapps/mobile/src/app/(tabs)/index.tsxapps/mobile/src/app/(tabs)/todos.tsxapps/mobile/src/app/_layout.tsxapps/mobile/src/app/index.tsxapps/mobile/src/app/locked.tsxapps/mobile/src/app/note/[id].tsxapps/mobile/src/app/notebooks.tsxapps/mobile/src/app/notes.tsxapps/mobile/src/app/trash.tsxapps/mobile/src/components/format-sheet.tsxapps/mobile/src/components/icon.tsxapps/mobile/src/components/sheet.tsxapps/mobile/src/db/migrations.tsapps/mobile/src/db/notebooks-repo.tsapps/mobile/src/db/notes-repo.tsapps/mobile/src/lib/format.tsapps/mobile/src/lib/lock.tsapps/mobile/src/lib/undo.tsapps/mobile/src/store/notebooks-store.tsxapps/mobile/src/store/notes-store.tsxapps/mobile/src/theme/colors.jsonapps/mobile/src/theme/index.tsapps/mobile/tailwind.config.jspackages/core/src/formatting.test.tspackages/core/src/formatting.tspackages/core/src/index.tspackages/core/src/note.tspackages/core/src/title.tspackages/core/src/types.ts
💤 Files with no reviewable changes (2)
- apps/mobile/src/app/notes.tsx
- apps/mobile/src/app/index.tsx
| tabBarStyle: { | ||
| backgroundColor: theme.canvas, | ||
| borderTopColor: theme.line, | ||
| borderTopWidth: 1, | ||
| height: 62, | ||
| paddingTop: 6, | ||
| elevation: 0, | ||
| }, | ||
| tabBarLabelStyle: { fontSize: 11, paddingBottom: 6 }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
React Navigation bottom tabs tabBarStyle height safe area insets bottom behavior
💡 Result:
In React Navigation, the bottom tab bar automatically calculates and applies safe area insets to its own height to ensure it is not obscured by system UI elements like the home indicator on iOS [1]. Key behavioral details: Automatic Handling: The built-in bottom tab bar uses react-native-safe-area-context to detect device insets [1][2]. When the tab bar is positioned at the bottom, it automatically adds the bottom safe area inset to its calculated height, preventing content overlap [1]. Customization and Styling: - If you use the standard tab bar, you should avoid manually overriding its layout to account for safe areas, as this can conflict with the internal automatic calculations [1][3]. - When implementing a custom tab bar via the tabBarStyle property, you are responsible for managing these insets. The library's internal logic calculates height based on a base height plus the bottom safe area inset [1]. - If you are setting a custom height in tabBarStyle, note that the tab bar's internal logic may still apply additional padding or inset logic depending on the implementation [1]. Managing Content Overlap: Because the tab bar occupies space at the bottom of the screen, your screen content may be hidden behind it. To manage this: - Use the useSafeAreaInsets hook from react-native-safe-area-context within your screens to apply appropriate padding to your containers, ensuring your content remains visible [2][4]. - On iOS, for scrollable lists (ScrollView, FlatList, etc.), use contentInsetAdjustmentBehavior="automatic" as a preferred, automated way to handle insets without manual padding calculations [2][4]. - Avoid wrapping your entire application in a single SafeAreaView; instead, apply safe area handling to the specific screen content that requires it [2][4]. Native Bottom Tabs Note: If you are using native bottom tabs, the tab bar is rendered outside the standard React Native JS layout tree [5]. Consequently, there is no direct equivalent to useBottomTabBarHeight, and you cannot measure the tab bar height directly from JS [5]. You must rely on safe area insets to apply necessary bottom padding to your content to prevent overlap [5].
Citations:
- 1: https://github.com/react-navigation/react-navigation/blob/main/packages/bottom-tabs/src/views/BottomTabBar.tsx
- 2: https://reactnavigation.org/docs/8.x/handling-safe-area
- 3: Bottom inset on a BottomTabNavigator is 4 pixels off on iOS react-navigation/react-navigation#11480
- 4: https://reactnavigation.org/docs/handling-safe-area/
- 5: [v8][native bottom tabs] useBottomTabBarHeight react-navigation/react-navigation#12949
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching _layout.tsx / bottom tab screens:\n'
fd -a '_layout\.tsx$|Tabs|tab' . | sed 's#^\./##' | head -80
printf '\nTarget file excerpt:\n'
cat -n "apps/mobile/src/app/(tabs)/_layout.tsx" | sed -n '1,120p'
printf '\nSearch for safe-area usage and screenOptions edges:\n'
rg -n "SafeAreaView|useSafeAreaInsets|edges=\\{|edges:" apps/mobile/src -S || trueRepository: zacktiger/DailyNote
Length of output: 1924
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect React Navigation dependency versions/package manifests and BottomTabBar/source if included.
printf 'Package references:\n'
rg -n '"react-navigation/bottom-tabs"|"react-navigation"|react-navigation-bottom-tabs|BottomTabBar' package.json apps/mobile/package.json . -S 2>/dev/null | head -120 || true
printf '\nSearch for local vendored BottomTabBar TypeScript:\n'
fd -a 'BottomTabBar\.tsx$|BottomTabBar\.js$' . | sed 's#^\./##' | head -50
# Read-only behavioral/source probe: parse local BottomTabBar if present to see how safeAreaInsetBottom affects height/padding.
python3 - <<'PY'
from pathlib import Path
roots = [Path('packages/bottom-tabs/src/views'), Path('apps/mobile/node_modules/react-navigation/bottom-tabs')]
found=[]
for p in roots:
for f in p.glob('BottomTabBar.tsx'):
found.append(f)
for f in p.glob('BottomTabBar.js'):
found.append(f)
if not found:
print('No local BottomTabBar source found in expected roots')
raise SystemExit
for f in found:
print(f'## {f}')
lines=f.read_text().splitlines()
start=max(0, next((i for i,l in enumerate(lines) if 'calculateHeight' in l or 'safeArea' in l or 'tabBarStyle' in l), 0)-20)
end=min(len(lines), start+140)
for i in range(start, end):
if any(t in lines[i] for t in ['safeAreaInsetBottom','calculateHeight','tabBarStyle','style={{','StyleProvider']):
print(f'{i+1:5d}: {lines[i]}')
PYRepository: zacktiger/DailyNote
Length of output: 1924
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching _layout.tsx / bottom tab screens:\n'
fd -a '_layout\.tsx$' . | sed 's#^\./##' | head -80
printf '\nTarget file excerpt:\n'
cat -n "apps/mobile/src/app/(tabs)/_layout.tsx" | sed -n '1,120p'
printf '\nSearch for safe-area usage and screenOptions edges:\n'
rg -n "SafeAreaView|useSafeAreaInsets|edges=\\{|edges:" apps/mobile/src -S || true
printf '\nPackage references:\n'
rg -n '"react-navigation/bottom-tabs"|"react-navigation"|react-navigation-bottom-tabs|BottomTabBar' package.json apps/mobile/package.json . -S 2>/dev/null | head -120 || true
printf '\nSearch for local vendored BottomTabBar TypeScript:\n'
fd -a 'BottomTabBar\.tsx$|BottomTabBar\.js$' . | sed 's#^\./##' | head -50
python3 - <<'PY'
from pathlib import Path
roots = [Path('packages/bottom-tabs/src/views'), Path('apps/mobile/node_modules/react-navigation/bottom-tabs')]
found=[]
for p in roots:
for f in p.glob('BottomTabBar.tsx'):
found.append(f)
for f in p.glob('BottomTabBar.js'):
found.append(f)
if not found:
print('No local BottomTabBar source found in expected roots')
raise SystemExit
for f in found:
print(f'## {f}')
lines=f.read_text().splitlines()
start=max(0, next((i for i,l in enumerate(lines) if 'calculateHeight' in l or 'safeArea' in l or 'tabBarStyle' in l), 0)-20)
end=min(len(lines), start+140)
for i in range(start, end):
if any(t in lines[i] for t in ['safeAreaInsetBottom','calculateHeight','tabBarStyle','style={{','StyleProvider']):
print(f'{i+1:5d}: {lines[i]}')
PYRepository: zacktiger/DailyNote
Length of output: 1924
Verify the fixed tab bar height against the bottom safe-area inset.
tabBarStyle.height is hard-coded to 62. If the navigator does not add insets.bottom on top of an explicit custom layout height, tab screens with edges={['top', 'left', 'right']} can render overlap the bottom safe area. Add insets.bottom with useSafeAreaInsets(), or remove the explicit height.
🤖 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 `@apps/mobile/src/app/`(tabs)/_layout.tsx around lines 22 - 30, The
tabBarStyle.height is hard-coded to 62 without accounting for the bottom safe
area inset, which can cause tab screens with restricted edges to overlap the
safe area. Use the useSafeAreaInsets() hook to retrieve the bottom inset and add
it to the hard-coded height value in the tabBarStyle object, or alternatively
remove the explicit height property and let the navigator handle the layout.
| const rootNotes = useMemo( | ||
| () => notes.filter((note) => note.kind === 'note' && !note.locked), | ||
| [notes], | ||
| ); | ||
|
|
||
| const inNotebook = useMemo(() => { | ||
| if (filter === ALL_NOTES) return rootNotes; | ||
| if (filter === DEFAULT_NOTEBOOK) return rootNotes.filter((note) => note.notebookId === null); | ||
| return rootNotes.filter((note) => note.notebookId === filter); | ||
| }, [rootNotes, filter]); | ||
|
|
||
| const results = useMemo(() => searchNotes(inNotebook, query), [inNotebook, query]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Both tab screens omit deleted and archivedAt from their visibility filter. Each screen re-derives its own predicate over the shared useNotes() state and checks only kind and locked. The store holds soft-deleted rows, which apps/mobile/src/app/trash.tsx reads from the same state, so trashed and archived notes leak into both views. Consider extracting one shared isVisible(note) predicate so a future flag cannot be missed at one site.
apps/mobile/src/app/(tabs)/index.tsx#L60-L71: add!note.deleted && note.archivedAt === nulltorootNotes, so the header count at Line 105 matches the rendered rows after a swipe-delete.apps/mobile/src/app/(tabs)/todos.tsx#L34-L37: add!note.deleted && note.archivedAt === nulltocommitments, so a deleted to-do leaves the Due, Later, and Done sections.
📍 Affects 2 files
apps/mobile/src/app/(tabs)/index.tsx#L60-L71(this comment)apps/mobile/src/app/(tabs)/todos.tsx#L34-L37
🤖 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 `@apps/mobile/src/app/`(tabs)/index.tsx around lines 60 - 71, Update
apps/mobile/src/app/(tabs)/index.tsx lines 60-71 in the rootNotes useMemo
predicate to also require !note.deleted and note.archivedAt === null, preserving
the existing kind and locked checks. Apply the same visibility filtering in
apps/mobile/src/app/(tabs)/todos.tsx lines 34-37 within commitments so deleted
or archived to-dos are excluded from all sections; prefer a shared
isVisible(note) predicate if the existing structure supports it.
| <Pressable | ||
| hitSlop={10} | ||
| accessibilityRole="checkbox" | ||
| accessibilityState={{ checked: complete }} | ||
| accessibilityLabel={`Mark ${note.title ?? 'note'} done`} | ||
| onPress={() => { | ||
| haptics.success(); | ||
| void setCompleted(note.id, !complete); | ||
| }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the checkbox label for the un-complete action.
accessibilityLabel is always Mark <title> done. When complete is true, the press un-completes the note, so the announced label describes the opposite action.
🐛 Proposed fix
accessibilityState={{ checked: complete }}
- accessibilityLabel={`Mark ${note.title ?? 'note'} done`}
+ accessibilityLabel={
+ complete
+ ? `Mark ${note.title ?? 'note'} not done`
+ : `Mark ${note.title ?? 'note'} done`
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Pressable | |
| hitSlop={10} | |
| accessibilityRole="checkbox" | |
| accessibilityState={{ checked: complete }} | |
| accessibilityLabel={`Mark ${note.title ?? 'note'} done`} | |
| onPress={() => { | |
| haptics.success(); | |
| void setCompleted(note.id, !complete); | |
| }} | |
| <Pressable | |
| hitSlop={10} | |
| accessibilityRole="checkbox" | |
| accessibilityState={{ checked: complete }} | |
| accessibilityLabel={ | |
| complete | |
| ? `Mark ${note.title ?? 'note'} not done` | |
| : `Mark ${note.title ?? 'note'} done` | |
| } | |
| onPress={() => { | |
| haptics.success(); | |
| void setCompleted(note.id, !complete); | |
| }} |
🤖 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 `@apps/mobile/src/app/`(tabs)/todos.tsx around lines 109 - 117, The
accessibilityLabel in the Pressable component with hitSlop=10 is static and
always announces "Mark <title> done", but the onPress handler toggles the
complete state in both directions. Update the accessibilityLabel to be
conditional based on the complete variable so that when complete is true, it
announces an action that marks the note incomplete (such as "Mark <title> not
done"), and when complete is false, it announces "Mark <title> done".
| useEffect(() => { | ||
| // The OS prompt is the external system here; the state lands in its | ||
| // callback rather than in the effect body. | ||
| let active = true; | ||
| void authenticate('Unlock your notes').then((result) => { | ||
| if (!active) return; | ||
| setGate(result.unavailable === true ? 'unavailable' : result.ok ? 'open' : 'locked'); | ||
| }); | ||
| return () => { | ||
| active = false; | ||
| }; | ||
| }, [attempt]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle a rejected authenticate promise.
authenticate calls LocalAuthentication.authenticateAsync, which can reject. The chain has no rejection handler, so the gate stays at 'checking'. Line 139 then renders null, and the screen shows only the header with no Unlock button. The user cannot recover without leaving the screen. Treat a rejection as 'locked' so the retry path stays reachable.
🐛 Proposed fix
let active = true;
- void authenticate('Unlock your notes').then((result) => {
- if (!active) return;
- setGate(result.unavailable === true ? 'unavailable' : result.ok ? 'open' : 'locked');
- });
+ void authenticate('Unlock your notes').then(
+ (result) => {
+ if (!active) return;
+ setGate(result.unavailable === true ? 'unavailable' : result.ok ? 'open' : 'locked');
+ },
+ () => {
+ if (!active) return;
+ setGate('locked');
+ },
+ );
return () => {
active = false;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| // The OS prompt is the external system here; the state lands in its | |
| // callback rather than in the effect body. | |
| let active = true; | |
| void authenticate('Unlock your notes').then((result) => { | |
| if (!active) return; | |
| setGate(result.unavailable === true ? 'unavailable' : result.ok ? 'open' : 'locked'); | |
| }); | |
| return () => { | |
| active = false; | |
| }; | |
| }, [attempt]); | |
| useEffect(() => { | |
| // The OS prompt is the external system here; the state lands in its | |
| // callback rather than in the effect body. | |
| let active = true; | |
| void authenticate('Unlock your notes').then( | |
| (result) => { | |
| if (!active) return; | |
| setGate(result.unavailable === true ? 'unavailable' : result.ok ? 'open' : 'locked'); | |
| }, | |
| () => { | |
| if (!active) return; | |
| setGate('locked'); | |
| }, | |
| ); | |
| return () => { | |
| active = false; | |
| }; | |
| }, [attempt]); |
🤖 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 `@apps/mobile/src/app/locked.tsx` around lines 36 - 47, Update the authenticate
promise chain in the useEffect for the locked screen to handle rejections by
setting the gate to 'locked', while still honoring the active guard to avoid
state updates after cleanup. Keep the existing success-result handling unchanged
so the Unlock retry path remains available when authenticate fails.
| const [title, setTitle] = useState(() => stored.split('\n')[0] ?? ''); | ||
| const history = useHistory(stored.split('\n').slice(1).join('\n')); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep newlines out of the title.
composed joins title and body with \n, and the next mount splits on the first newline. The title input is multiline, so a pasted multi-line string puts newlines into title. Text then moves from the title into the body when the note is reopened. submitBehavior="submit" blocks Enter but not paste. Normalize the title on change.
🐛 Proposed fix
const stored = note?.body ?? '';
- const [title, setTitle] = useState(() => stored.split('\n')[0] ?? '');
+ const [title, setTitle] = useState(() => stored.split('\n')[0] ?? '');
+ const onChangeTitle = useCallback((next: string) => {
+ // The title is the body's first line, so it cannot contain a newline.
+ setTitle(next.replace(/\r?\n/g, ' '));
+ }, []); <TextInput
value={title}
- onChangeText={setTitle}
+ onChangeText={onChangeTitle}
placeholder="Heading"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [title, setTitle] = useState(() => stored.split('\n')[0] ?? ''); | |
| const history = useHistory(stored.split('\n').slice(1).join('\n')); | |
| const [title, setTitle] = useState(() => stored.split('\n')[0] ?? ''); | |
| const onChangeTitle = useCallback((next: string) => { | |
| // The title is the body's first line, so it cannot contain a newline. | |
| setTitle(next.replace(/\r?\n/g, ' ')); | |
| }, []); | |
| const history = useHistory(stored.split('\n').slice(1).join('\n')); |
🤖 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 `@apps/mobile/src/app/note/`[id].tsx around lines 92 - 93, Normalize the title
in the title input’s change handler so newline characters are removed or
truncated before updating the title state. Update the handler associated with
title and setTitle, preserving the existing composed serialization and body
behavior while ensuring pasted multiline text cannot move into the body on
reopen.
| useEffect(() => { | ||
| let active = true; | ||
| repo | ||
| .all() | ||
| .then((rows) => { | ||
| if (active) setNotebooks(rows); | ||
| }) | ||
| .finally(() => { | ||
| if (active) setLoading(false); | ||
| }); | ||
| return () => { | ||
| active = false; | ||
| }; | ||
| }, [repo]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Add a rejection handler to the initial load.
repo.all() has a .then and a .finally but no .catch. If the read fails, the promise rejects with no handler. React Native reports an unhandled promise rejection, notebooks stays empty, and loading still clears, so the UI presents an empty notebook list as a successful load.
Handle the rejection and keep the failure visible in the logs.
♻️ Proposed fix
useEffect(() => {
let active = true;
repo
.all()
.then((rows) => {
if (active) setNotebooks(rows);
})
+ .catch((error: unknown) => {
+ // An empty list is indistinguishable from a failed read, so record why.
+ console.error('Failed to load notebooks', error);
+ })
.finally(() => {
if (active) setLoading(false);
});
return () => {
active = false;
};
}, [repo]);apps/mobile/src/store/notes-store.tsx:58-71 has the same shape. Consider fixing both, or extracting the load pattern into one helper.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| let active = true; | |
| repo | |
| .all() | |
| .then((rows) => { | |
| if (active) setNotebooks(rows); | |
| }) | |
| .finally(() => { | |
| if (active) setLoading(false); | |
| }); | |
| return () => { | |
| active = false; | |
| }; | |
| }, [repo]); | |
| useEffect(() => { | |
| let active = true; | |
| repo | |
| .all() | |
| .then((rows) => { | |
| if (active) setNotebooks(rows); | |
| }) | |
| .catch((error: unknown) => { | |
| // An empty list is indistinguishable from a failed read, so record why. | |
| console.error('Failed to load notebooks', error); | |
| }) | |
| .finally(() => { | |
| if (active) setLoading(false); | |
| }); | |
| return () => { | |
| active = false; | |
| }; | |
| }, [repo]); |
🤖 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 `@apps/mobile/src/store/notebooks-store.tsx` around lines 47 - 60, The initial
load in the notebooks store lacks rejection handling. Add a catch handler to the
repo.all() promise chain in the useEffect, logging the failure while preserving
the active guard and loading cleanup; apply the same fix to the analogous
initial load in notes-store.tsx.
| async remove(id) { | ||
| await repo.remove(id); | ||
| await refresh(); | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
remove leaves the notes store stale, and every caller must remember to fix it.
repo.remove(id) re-files the notebook's notes by setting notes.notebook_id = null (apps/mobile/src/db/notebooks-repo.ts:116-130). This method then refreshes only the notebooks. The notes held by NotesProvider still carry the removed notebookId.
apps/mobile/src/app/notebooks.tsx:85-109 works around this by calling refreshNotes() after remove(notebook.id), and its comment states that the notes store is stale until it re-reads. That makes a data invariant the caller's obligation. Any future caller of remove that omits the extra refresh renders notes filed under a notebook that no longer exists, and the notebook filter rail in apps/mobile/src/app/(tabs)/index.tsx then shows a chip-less bucket.
Make remove own the full invalidation.
♻️ Two options to move the invalidation into the store
Option 1 — accept a refresh callback, keeping the providers independent:
- async remove(id) {
- await repo.remove(id);
- await refresh();
- },
+ // remove() re-files the notebook's notes, so both stores are stale.
+ // The caller supplies the notes refresh because NotebooksProvider sits
+ // above NotesProvider and cannot read its context.
+ async remove(id, refreshNotes) {
+ await repo.remove(id);
+ await Promise.all([refresh(), refreshNotes()]);
+ },Update the contract at Line 24:
- remove: (id: string) => Promise<void>;
+ /** Also re-files the notebook's notes, so the notes store must re-read. */
+ remove: (id: string, refreshNotes: () => Promise<void>) => Promise<void>;Option 2 — nest NotebooksProvider inside NotesProvider in apps/mobile/src/app/_layout.tsx and call useNotes().refresh() directly inside remove. This removes the parameter but inverts the current provider order, so check that nothing in NotesProvider reads notebooks.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async remove(id) { | |
| await repo.remove(id); | |
| await refresh(); | |
| }, | |
| async remove(id, refreshNotes) { | |
| await repo.remove(id); | |
| await Promise.all([refresh(), refreshNotes()]); | |
| }, |
🤖 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 `@apps/mobile/src/store/notebooks-store.tsx` around lines 95 - 98, Update the
NotebooksProvider remove method to own notes invalidation after repo.remove(id),
using the chosen refresh-callback or provider-nesting approach so both notebooks
and notes refresh before returning. Then remove the redundant refreshNotes
workaround from the notebooks screen and preserve the existing provider contract
consistently for all callers.
| "faint": { "light": "#8d887e", "dark": "#6a6763" }, | ||
| "line": { "light": "#e2ded4", "dark": "#302f2c" }, | ||
| "accent": { "light": "#b0740a", "dark": "#f0a92b" }, | ||
| "accentInk": { "light": "#ffffff", "dark": "#141310" }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Check the text contrast for faint and accentInk.
faint carries informational text, not decoration. Examples: the empty-state hint in apps/mobile/src/app/(tabs)/index.tsx line 375, the deleted-date line in apps/mobile/src/app/trash.tsx, and placeholderTextColor in the search field. accentInk carries the label on accent-filled buttons.
faint light #8d887e on canvas light #f7f5f0, and faint dark #6a6763 on canvas dark #000000, both look below the WCAG AA 4.5:1 minimum for text under 18.66px. Run the script to confirm, then darken the light value and lighten the dark value as needed.
#!/bin/bash
# Compute WCAG contrast ratios for the palette pairs used for text.
fd -t f 'colors.json' --glob '**/theme/**' --exec python3 - {} <<'PY'
import json, sys
palette = json.load(open(sys.argv[1]))
def lin(c):
c = c / 255
return c / 12.92 if c <= 0.03928 else ((c + 0.055) / 1.055) ** 2.4
def lum(hexstr):
h = hexstr.lstrip('#')
r, g, b = (int(h[i:i+2], 16) for i in (0, 2, 4))
return 0.2126 * lin(r) + 0.7152 * lin(g) + 0.0722 * lin(b)
def ratio(fg, bg):
a, b = lum(fg), lum(bg)
hi, lo = max(a, b), min(a, b)
return (hi + 0.05) / (lo + 0.05)
pairs = [
('faint', 'canvas'), ('faint', 'card'), ('muted', 'canvas'),
('ink', 'canvas'), ('accent', 'canvas'), ('accentInk', 'accent'),
]
for fg, bg in pairs:
for scheme in ('light', 'dark'):
r = ratio(palette[fg][scheme], palette[bg][scheme])
print(f'{scheme:5} {fg:9} on {bg:7} {r:5.2f}:1 AA-text={"pass" if r >= 4.5 else "FAIL"}')
PY🤖 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 `@apps/mobile/src/theme/colors.json` around lines 9 - 12, Update the faint and
accentInk color values in the theme palette to meet WCAG AA contrast: darken
faint.light against canvas.light, lighten faint.dark against canvas.dark, and
adjust accentInk values if needed for readable text on accent backgrounds.
Verify all affected palette pairs with the provided contrast-check script,
preserving the existing light/dark theme structure.
| it('preserves ticks when a checklist is re-applied over itself', () => { | ||
| const { text, selection } = at('|- [x] one\n- [ ] two|'); | ||
| expect(applyListStyle(text, selection, 'bullet').text).toBe('- one\n- two'); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The test name does not match the assertion.
The name says a checklist is re-applied over itself and the ticks are preserved. The body applies 'bullet' to a checklist and asserts the ticks are dropped, which is the opposite behavior. applyListStyle sets checked: next === 'checklist' ? line.checked : false, so the assertion is correct and the name is stale.
Rename the test, and add the case the current name describes.
💚 Proposed fix for the test name and the missing case
- it('preserves ticks when a checklist is re-applied over itself', () => {
+ it('drops ticks when a checklist becomes a bullet list', () => {
const { text, selection } = at('|- [x] one\n- [ ] two|');
expect(applyListStyle(text, selection, 'bullet').text).toBe('- one\n- two');
});
+
+ it('preserves ticks when a checklist is re-applied over itself', () => {
+ const { text, selection } = at('|- [x] one\n- [ ] two|');
+ expect(applyListStyle(text, selection, 'checklist').text).toBe('- [x] one\n- [ ] two');
+ });Confirm the second expectation against the implementation: re-applying 'checklist' toggles the list to 'none', because listStyleAt already reports 'checklist'. Adjust the expectation to the intended toggle behavior.
🤖 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 `@packages/core/src/formatting.test.ts` around lines 166 - 169, Rename the
existing test to describe applying bullet styling to a checklist and dropping
its ticks. Add a separate test covering reapplying checklist styling to an
existing checklist, asserting the intended toggle-to-none behavior reported by
listStyleAt and the resulting unchecked/list output.
| export function hasInlineMark(text: string, selection: Selection, mark: InlineMark): boolean { | ||
| const [open, close] = INLINE_MARKERS[mark]; | ||
| const start = Math.min(selection.start, selection.end); | ||
| const end = Math.max(selection.start, selection.end); | ||
| const inner = text.slice(start, end); | ||
|
|
||
| if (inner.startsWith(open) && inner.endsWith(close) && inner.length >= open.length + close.length) { | ||
| return true; | ||
| } | ||
| return text.slice(start - open.length, start) === open && text.slice(end, end + close.length) === close; | ||
| } | ||
|
|
||
| /** | ||
| * Wraps the selection in a mark, or unwraps it when already marked. | ||
| * | ||
| * With an empty selection this inserts the marker pair and puts the caret | ||
| * between them, so tapping B and typing produces bold text. | ||
| */ | ||
| export function toggleInlineMark(text: string, selection: Selection, mark: InlineMark): Edit { | ||
| const [open, close] = INLINE_MARKERS[mark]; | ||
| const start = Math.min(selection.start, selection.end); | ||
| const end = Math.max(selection.start, selection.end); | ||
| const inner = text.slice(start, end); | ||
|
|
||
| // Markers inside the selection. | ||
| if (inner.startsWith(open) && inner.endsWith(close) && inner.length >= open.length + close.length) { | ||
| const stripped = inner.slice(open.length, inner.length - close.length); | ||
| return { | ||
| text: `${text.slice(0, start)}${stripped}${text.slice(end)}`, | ||
| selection: { start, end: start + stripped.length }, | ||
| }; | ||
| } | ||
|
|
||
| // Markers hugging the selection. | ||
| if ( | ||
| text.slice(start - open.length, start) === open && | ||
| text.slice(end, end + close.length) === close | ||
| ) { | ||
| return { | ||
| text: `${text.slice(0, start - open.length)}${inner}${text.slice(end + close.length)}`, | ||
| selection: { start: start - open.length, end: end - open.length }, | ||
| }; | ||
| } | ||
|
|
||
| const wrapped = `${open}${inner}${close}`; | ||
| return { | ||
| text: `${text.slice(0, start)}${wrapped}${text.slice(end)}`, | ||
| selection: | ||
| start === end | ||
| ? { start: start + open.length, end: start + open.length } | ||
| : { start, end: start + wrapped.length }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Italic detection matches bold markers, so italic removes bold.
INLINE_MARKERS.italic is ['*', '*'], and both hasInlineMark and toggleInlineMark test only startsWith(open) / endsWith(close) plus a length floor. A selection of **bold** satisfies that test for italic: length 8 is at least 2, and the run starts and ends with *.
Two consequences are reachable from the editor toolbar in apps/mobile/src/app/note/[id].tsx:
hasInlineMark(text, selection, 'italic')returnstruefor bold text, so the italic control renders as active.toggleInlineMark(text, selection, 'italic')strips one*from each end.**bold**becomes*bold*. The bold mark is replaced by italic instead of italic being added.
toggleInlineMark already leaves the whole wrapped run selected after a bold toggle (Line 407), so tapping B then I on the same selection reaches this in two taps.
Exclude a doubled marker when the mark is italic.
🐛 Proposed fix to keep italic from matching bold
+/**
+ * True when `run` is wrapped in exactly `open`/`close` and not in a longer
+ * marker made of the same character -- `**bold**` is not italic.
+ */
+function isWrappedIn(run: string, open: string, close: string): boolean {
+ if (!run.startsWith(open) || !run.endsWith(close)) return false;
+ if (run.length < open.length + close.length) return false;
+ const inner = run.slice(open.length, run.length - close.length);
+ return !inner.startsWith(open[0]!) && !inner.endsWith(close[0]!);
+}
+
/** True when the selection is already wrapped in the given mark. */
export function hasInlineMark(text: string, selection: Selection, mark: InlineMark): boolean {
const [open, close] = INLINE_MARKERS[mark];
const start = Math.min(selection.start, selection.end);
const end = Math.max(selection.start, selection.end);
const inner = text.slice(start, end);
- if (inner.startsWith(open) && inner.endsWith(close) && inner.length >= open.length + close.length) {
- return true;
- }
+ if (isWrappedIn(inner, open, close)) return true;
return text.slice(start - open.length, start) === open && text.slice(end, end + close.length) === close;
}Apply the same helper in toggleInlineMark at Line 382. Add a test asserting toggleInlineMark('**bold**', { start: 0, end: 8 }, 'italic').text is '***bold***' (or whatever nesting you choose), and that hasInlineMark for italic over **bold** is false.
🤖 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 `@packages/core/src/formatting.ts` around lines 357 - 409, Update hasInlineMark
and toggleInlineMark to exclude doubled '*' runs when mark is italic, so bold
text is not detected as italic and toggling italic wraps rather than strips bold
markers. Reuse a shared helper for both functions, and add coverage for
hasInlineMark on **bold** returning false and toggleInlineMark producing the
intended nested italic result.
Both branches solved rich text, differently. origin/main's block document won: it is a real structural model with a versioned `doc` column, `body` kept as its plain-text projection so search, hashtags, deriveTitle and the Phase 4 sync cursor are untouched -- and it has working photo attachments, which the plain-text approach could not express at all. So packages/core/formatting.ts and its 69 tests are gone. What they did -- headings, lists, alignment via Markdown-ish markers in one TextInput -- the block model does properly, per block, with the structure stored rather than typed into the text. Resolved: - migrations: theirs stays v2 (doc); notebooks/locked/pinned move to v3. Renumbering rather than merging keeps both upgrade paths intact for a database that has already applied v2. - notes-repo, types, note.ts: union of both column sets. - note/[id]: this redesign's chrome (back/undo/redo/overflow/done, the meta line, the notebook and note menus) wrapped around their BlockEditor. Undo became generic over its value, since content is now a Block[] and every edit already produces a new one. - format-sheet: rebacked by block operations. It now offers exactly what the document can express -- bullets, alignment, images -- instead of controls with nowhere to be stored. - block-editor: moved onto the new type scale and given the amber caret. - block-toolbar: deleted. It hand-drew its alignment icons because the app shipped no icon set; it does now, and the rail replaced it. - app/index.tsx: deleted. The list is the front door; the composer route it modified no longer exists. Also fixes back with no history: entering a note by deep link, share intent or a reloaded web URL left router.back() a no-op with the user stranded. useGoBack falls back to the list. Splash colours follow the new palette; they were still the old near-black that made the app look like it had not started. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Summary by CodeRabbit