Develop #567
Merged
Develop #567
Conversation
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
- Remove var(--spacing-md) margin from blockquote, use 0 !important - Remove trailing newline in markdown renderer that caused extra spacing - Ensure all blockquote children have no margin/padding - Prevent margin-top collapse on elements after blockquote - Set display: block explicitly for consistent block behavior
- Set margin: 0 on pre, callout, ul/ol, li, table, img, hr, headings - Set margin: 0 on .cm-block-code for CodeMirror editor - Ensures consistent spacing in StaticMarkdownRenderer context - Prevents margin collapse issues in block element sequences
- Explicitly call contentDOM.blur() when isFocused becomes false - Fixes data loss when clicking to another block before editor blurs naturally - Ensures onBlur handler and commitDraft are always triggered - Prevents rapid focus switching from losing unsaved changes
- Add explicit useEffect to call commitDraft() when isFocused becomes false - This runs BEFORE the sync useEffect that resets draftRef to blockContent - Prevents data loss when rapidly switching focus between blocks - Ensures edited content is saved, not the original block content
- Add isDirtyRef to track unsaved edits per block - Guard sync effect to skip when draft is dirty and not programmatic nav - Mark draft as dirty on any content change - Mark draft as clean after successful commit - Prevents sync effect from overwriting draft before commitDraft runs This fixes the race condition where clicking between blocks would cause unsaved changes to disappear. The problem was that multiple useEffect hooks (sync + commit) both depend on isFocused changing, and they race to execute. The sync effect would reset draftRef to original blockContent before commitDraft could save the edited content. With isDirtyRef guard, the sync effect now safely skips on focus loss if there are unsaved changes, allowing commitDraft to complete first.
…ntent The core issue: when focusing between blocks rapidly, sync effect could reset draftRef to original blockContent before commitDraft completed. Solution: only sync when blockContent === draftRef.current - If they differ, there are unsaved edits → skip sync, let commit finish - After commit completes, next sync will apply any external updates - Programmatic nav still overrides (for merge/split/move operations) This eliminates the race condition without requiring a separate dirty flag, making the logic simpler and more robust.
…e click Key improvements: - Fixed race condition where unfocused blocks override their drafts before store updates complete - Changed override logic from 'isProgrammaticNav && !isFocused' to only trigger for target block of navigation - Added guard 'blockContent !== ""' to skip override when store values are stale - Added comprehensive logging throughout focus, commit, and content sync lifecycle Root cause: When user clicks block B while editing block A, the Tauri save invoke is async. Before it completes, block A's blockContentEffect fires and overrides the unsaved draft with empty/stale blockContent from store, losing the user's edits. Fix ensures: Only the NEWLY FOCUSED block (isTargetOfNav) can override its draft during programmatic nav. Other blocks that lose focus are protected from override. Also fixes empty block clickability (Issue 2): - Added min-width and min-height to .block-content-wrapper and .block-row - Added cursor:text for better UX - Prevents empty blocks from being too small to click
Clean up console logging added during investigation phase. Core logic remains unchanged. Keeps one diagnostic log in updateBlockContent START for debugging if needed in production, but removes verbose state tracking logs to keep output clean.
Document the subtle async/concurrency issue that the block content synchronization guards prevent. Explains: - Problem: Tauri invoke is async, stale values can arrive mid-focus switch - Solution: Three-case logic with blockContent !== '' guard - Tradeoff: Rare edge case (clearing block while another saves) accepted This docstring is essential to prevent future maintainers from trying to 'fix' the apparently strange guards and reintroducing the bug.
Create RACE_CONDITION_FIX.md with: - Problem explanation and root cause analysis - Detailed solution approach with code flow - Why alternative solutions were rejected - Complete testing verification guide - Production readiness checklist This documentation captures the full context of the fix so future maintainers understand the pragmatic tradeoff and can make informed decisions about potential refactors.
- Extract BreadcrumbItem component to eliminate duplication - Replace direct setState calls with updateZoomPath() action - Fix React key generation to prevent duplicate page name issues - Add ARIA labels and semantic HTML (nav, ol, li) for accessibility - Add error handling and loading state for navigation - Extract constants (BREADCRUMB_MAX_LENGTH, CHEVRON_SIZE, CHEVRON_OPACITY) - Improve CSS with theme variables and proper disabled/focus states - Remove unused pageName props from Breadcrumb and PageHeader - Add explicit null/undefined checks for type safety
- Fix double highlighting: only highlight current page when zoom is empty - Add handleZoomOutToPage to clear zoom when navigating to pages - Clear zoom path when page is clicked in breadcrumb - Distinguish between page hierarchy (non-zoomed) and block hierarchy (zoomed) - Only show last item as current when not in zoom mode - Smooth transition between page and block hierarchy levels
- Refactor BlockComponent to use zoomToBlock() action instead of direct setState - Implement BlockEditor zoom filtering based on zoomPath state - Update Breadcrumb to use new zoom actions (zoomOutToIndex, clearZoom) - Add per-page zoom persistence via zoomByPageId in viewStore - Zoom now properly filters which blocks display in the editor - Fixes critical bug where zoom wasn't reflecting in BlockEditor
When zooming into a block, the block should be displayed along with its children via the recursive rendering in blockOrder. Previously only the children were shown, causing the zoomed block to disappear.
BREAKING: zoom now works correctly via parent-child relationships Changes: - zoomToBlock() now calculates full ancestor chain instead of simple push - Traverses parentId upward to build [root, parent, child] path - Enables correct zoom-out navigation - Fixed zoomByPageId to use array copies instead of references - All zoom actions (zoomToBlock, zoomOutToIndex, zoomOut, goBack) now copy arrays - Prevents subtle synchronization bugs on page navigation - Added auto-recovery in BlockEditor for invalid zoom targets - If zoomed block is deleted/missing, automatically clear zoom - Prevents empty screen when zoom target becomes invalid - Logs warning for debugging Fixes: - Zoom no longer loses context when zooming out - Blocks no longer disappear when zoom target is deleted - Per-page zoom persistence now works correctly
Critical fix for zoom persistence across page changes. Problem: When navigating away from a page and returning, the zoomed blocks would disappear because: 1. showPage() would restore zoomPath from zoomByPageId 2. But blocksById hadn't been updated yet (still had old page's blocks) 3. BlockEditor's auto-recovery would see zoom target missing and clear zoom 4. Result: User returns to page but zoom state is lost Solution: - showPage() no longer restores zoom (defer it) - BlockEditor now has separate useEffect that: 1. Waits for currentPageId to match and blocksById to be populated 2. Then restores zoom from zoomByPageId if it was previously set 3. Only then does auto-recovery validate the zoom target This ensures blocksById is ready before zoom restoration happens.
…condition Prevents auto-recovery from clearing zoom that was just restored on page navigation. Uses ref flag to skip one validation cycle after zoom restoration completes.
Two critical fixes for hierarchical blocks: 1. Safe reindex strategy: Instead of blindly deleting all blocks when markdown file timestamp changes, now preserves blocks in DB that are newer than the markdown file (likely pending sync). Only deletes blocks that are old and not in the markdown file. 2. Child block indentation: When syncing newly created child blocks to markdown, now properly determines indent level by: - First checking next sibling (same parent) - Then checking previous sibling (same parent) - Finally checking parent block's indent level (new!) - Previously defaulted to 0 if no siblings, causing children to appear at root Fixes issue where creating a child block, navigating away, and returning would show the blocks as deleted.
Previously, nested blocks were being returned separately in childrenByParent map but only rootBlocks were being processed by the frontend, causing nested blocks to disappear when navigating away from a page and returning. This fix simplifies the data flow by including all blocks (root and nested) directly in rootBlocks array. The existing normalizeBlocks() function already handles the hierarchy correctly based on parentId relationships, so there is no need to maintain a separate structure. Resolves the issue where depth > 0 blocks were saved to the database but failed to load on page re-entry. Ultraworked with Sisyphus
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.
Description
Describe the changes in this PR and what problem they solve.
Type of Change
Related Issues
Closes #
Changes Made
List the specific changes made in this PR:
Testing
How was this change tested?
Checklist
Screenshots (if applicable)
If this includes UI changes, add screenshots or videos.
Additional Information
Any other relevant information or context.