{{ message }}
Repository navigation
Conversation
Tux69140
force-pushed
the
feature/issue-1869-fold-collapse-text
branch
from
June 1, 2026 05:16
54158d2 to
622d71e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds heading section folding controls (chevrons + fold/unfold actions) to both the Muya realtime (WYSIWYG) editor and the CodeMirror 5 source-code editor, addressing #1869 by making folds visible and directly actionable in the editor body/gutter.
Changes:
- Introduces heading folding state and rendering in Muya (fold toggle icon, hide folded blocks, unfold-on-search/scroll integration).
- Adds CodeMirror fold gutter styling/behavior (chevrons, hover/active visibility) plus fold/unfold-all commands and menu/keybindings.
- Adds Playwright e2e coverage and updates end-user/dev documentation + localization strings for the new feature and commands.
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 3 comments.
Show a summary per file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+46
to
+61
| ContentState.prototype.getFoldHeadingForBlock = function(block) { | ||
| if (!block) return null | ||
|
|
||
| const outmostBlock = block.parent ? this.findOutMostBlock(block) : block | ||
| let heading = this.getBlock(outmostBlock.preSibling) | ||
|
|
||
| while (heading) { | ||
| if (this.isHeadingFolded(heading) && this.getSectionBlocks(heading).includes(outmostBlock)) { | ||
| return heading | ||
| } | ||
|
|
||
| heading = this.getBlock(heading.preSibling) | ||
| } | ||
|
|
||
| return null | ||
| } |
Comment on lines
+1220
to
+1228
| h1.ag-paragraph:hover .ag-front-icon, | ||
| h2.ag-paragraph:hover .ag-front-icon, | ||
| h3.ag-paragraph:hover .ag-front-icon, | ||
| h4.ag-paragraph:hover .ag-front-icon, | ||
| h5.ag-paragraph:hover .ag-front-icon, | ||
| h6.ag-paragraph:hover .ag-front-icon, | ||
| .ag-paragraph.ag-heading-folded-block:not(.ag-active) .ag-front-icon { | ||
| display: block; | ||
| } |
Comment on lines
882
to
892
| const scrollToHeader = (slug: unknown) => { | ||
| if (typeof slug !== 'string') return | ||
|
|
||
| const didUnfold = editor.value.unfoldBlockByKey(slug) | ||
| if (didUnfold) { | ||
| requestAnimationFrame(() => scrollToElement(`#${slug}`)) | ||
| return | ||
| } | ||
|
|
||
| return scrollToElement(`#${slug}`) | ||
| } |
Author
|
thanks, and sorry, I'm new in code process. |
Respond to Copilot review comments for the heading folding PR by optimizing Muya fold lookup during render, wiring the showHeadingFoldChevrons preference into WYSIWYG CSS, and preventing source-mode TOC navigation from mutating hidden Muya fold state. Also harden the dev-session restore path discovered while validating the review fixes: skip unreadable buffer files without deleting them, merge only duplicate dev buffer stores, preserve restore warning mappings after tab id remapping, and fall back to normal restore if compaction fails. Validation: pnpm -C packages/desktop run typecheck; pnpm -C packages/desktop exec eslint src/main/app/index.ts src/main/editorBufferStore/index.ts ../../packages/muyajs/lib/contentState/foldCtrl.js ../../packages/muyajs/lib/contentState/index.js.
Contributor
|
I've been looking into #1869 as well and wanted to share some findings before potentially duplicating work. Analysis of the current situation@Tux69140 — nice work getting both WYSIWYG and source-code mode fold working. I noticed this PR currently has merge conflicts with I did a deep dive into the codebase and found:
Proposed approachI'd recommend an incremental path: Phase 1 — WYSIWYG heading fold (new muya engine)
Phase 2 — Source Code mode
Phase 3 — Extended fold targets (future, separate PRs)
Open questionsBefore I start on this, a few questions for @Jocs:
How I see this fitting together@Tux69140 — if the maintainers prefer targeting the new muya engine, your source-code mode work (the CodeMirror fold gutter part) could still be valuable as a standalone PR since that's engine-independent. If legacy muyajs support is wanted, your WYSIWYG implementation is directly applicable. I'm happy to take on the new-muya implementation if that's the preferred direction, coordinate with @Tux69140 on the CM5 part, or step back if you'd rather own this end-to-end. Whatever works best for the project. I also have a few other PRs open (#4345, #4999, #5134, #5136, #5137, #5139) — would appreciate a review on those when you get a chance, @Jocs. Happy to address any feedback. |
Author
|
Hello Remi @rkristelijn , thank you for your detailed feedback. I don't have enough time right now to finish this project, so I'd be happy for you to take over. Thank you for your suggestion and your question. @Jocs , I'm having the same time-related issue finishing up my toolbar PR, which I started revising for the new version of the engine. I'll come later to see how can I help to Marktext. Blessings |
Member
|
Please resolve the conflicts before I can take a PR review. Thanks. |
Contributor
rkristelijn
added a commit
to rkristelijn/marktext-fork
that referenced
this pull request
Oct 2, 2026
Implements Option A from the fold design doc (marktext#1869): fold the section owned by an ATX heading in the new muya engine. A section is every block following a heading up to the next heading of equal-or-higher level, so folding an h1 also hides nested h2/h3. - foldSection.ts: pure, DOM-free section-resolution helpers (unit-tested) - HeadingFoldToggle: chevron affordance (attachment on AtxHeading), operable by click and keyboard (Enter/Space), with aria-expanded and accessible labels - AtxHeading.toggleFold()/_applyFold(): walk siblings and toggle the MU_FOLDED_CONTENT class; fold is runtime-only, never serialized - content.ts: beforeKeydown hook so headings can consume Cmd/Ctrl+Shift+[ (VS Code fold shortcut) before default key routing - CSS chevron + folded-content hiding; locales for all 11 languages - docs/design: design doc, PR marktext#4331 review note, and testing notes Verified: tsc --noEmit clean; 14 unit tests pass (foldSection + headingFold). Known open items (not yet resolved): - Affordance placement is provisional (copy-link -64px, chevron -40px, both in the left gutter). 'Copy-link directly after heading text' was reverted because width:fit-content on headings froze the renderer (see docs/design/fold-heading-testing-notes.md). - Hang fix reverted in source but not yet verified via a dev-server reload. - Collapse/expand children + document-wide fold (Option D) not started.
Jocs
added a commit
that referenced
this pull request
Oct 19, 2026
* feat(muya): add heading fold/unfold in WYSIWYG editor Implements Option A from the fold design doc (#1869): fold the section owned by an ATX heading in the new muya engine. A section is every block following a heading up to the next heading of equal-or-higher level, so folding an h1 also hides nested h2/h3. - foldSection.ts: pure, DOM-free section-resolution helpers (unit-tested) - HeadingFoldToggle: chevron affordance (attachment on AtxHeading), operable by click and keyboard (Enter/Space), with aria-expanded and accessible labels - AtxHeading.toggleFold()/_applyFold(): walk siblings and toggle the MU_FOLDED_CONTENT class; fold is runtime-only, never serialized - content.ts: beforeKeydown hook so headings can consume Cmd/Ctrl+Shift+[ (VS Code fold shortcut) before default key routing - CSS chevron + folded-content hiding; locales for all 11 languages - docs/design: design doc, PR #4331 review note, and testing notes Verified: tsc --noEmit clean; 14 unit tests pass (foldSection + headingFold). Known open items (not yet resolved): - Affordance placement is provisional (copy-link -64px, chevron -40px, both in the left gutter). 'Copy-link directly after heading text' was reverted because width:fit-content on headings froze the renderer (see docs/design/fold-heading-testing-notes.md). - Hang fix reverted in source but not yet verified via a dev-server reload. - Collapse/expand children + document-wide fold (Option D) not started. * refactor(muya): extract fold helpers, add collapsed-section marker - Extract isFoldShortcut() as a pure, DOM-free helper in foldSection.ts so the modifier chord logic is unit-testable without a real KeyboardEvent. - Extract toFoldSibling() to name the sibling→shape projection used by collectSectionIndices, with a comment explaining why non-heading blocks resolve to undefined level. - Extract _ownerHeading() in HeadingFoldToggle to name and document the defensive guard against stray clicks during mount/unmount. - Extract _setFoldedClass() in AtxHeading to make fold CSS toggling read as intent rather than DOM bookkeeping. - Add collapsed-section marker: a CSS ::after pill ("…") on folded headings so hidden content is discoverable. Uses a pseudo-element that does NOT set any intrinsic width on the heading — avoids the documented width:fit-content renderer freeze (see fold-heading-testing-notes.md). - 8 new tests (7 isFoldShortcut chord tests + 1 marker-class integration test); total fold tests: 22 passed, tsc --noEmit clean. - Updated testing notes with marker re-test checklist and verification status. Verified: tsc --noEmit clean; 22 unit+integration tests pass. Shortcut conflict: confirmed Cmd/Ctrl+Shift+[ is not bound elsewhere. Hang regression: no fit-content/max-content rule remains on headings (static); runtime freeze absence confirmed via dev-server (editor responsive, marker renders, main.log clean). * feat(muya): document-wide heading fold + clickable collapsed-section marker Builds on the per-heading fold with the actions users need to structure a document, and makes hidden content both visible and directly recoverable. - Clickable "…" collapsed-section marker (HeadingFoldMarker attachment): a real DOM element (a pseudo-element cannot receive a click) shown only while folded, keyboard + pointer operable, that unfolds its section. Positioned without any intrinsic heading width to avoid the documented fit-content renderer freeze. - Document-wide fold on AtxHeading: foldAll (every heading), unfoldAll, and foldToLevel(n) (fold headings deeper than n) driven by the pure, unit-tested foldPlanForLevel helper. foldAll is deliberately NOT foldToLevel(1). - Heading-only fold actions in the paragraph front menu (the block handle): Fold all / Unfold all / Fold to this level; shown only for atx-heading. - isFoldShortcut extracted to a pure helper; attachment lookup deduped into _attachmentByName; fold class toggle named _setFoldedClass (complexity hiding). - Locales for all 11 languages; CSS for the marker; 28 new/updated tests. Verified: tsc --noEmit clean; 50 fold unit+integration tests pass; front menu shows fold items only on headings; runtime dev-server check (fold/unfold, marker click, menu) OK, main.log clean. * fix(muya): keep folded nested sections hidden when unfolding an ancestor + lint CodeRabbit review of #5276/#4 caught a real nested-fold bug and lint failures. - Fix nested fold state (functional bug): unfolding an ancestor heading no longer reveals content owned by a still-folded descendant heading. _applyFold now consults a pure hiddenByFoldedDescendant() helper so a block stays hidden when it belongs to a nested heading that is itself folded. Regression test added (fold inner → fold ancestor → unfold ancestor). - Replace the three `as unknown as AtxHeading` double casts with a shared structural type guard isFoldableHeading() (foldSection.ts) — removes the lint-suppressed casts flagged by no-restricted-syntax, and _siblingHeadings / both _ownerHeading now narrow safely. - Lint fixes flagged by the muya CI (which was not covered by the desktop lint-staged hook): type-before-value import order, function-declaration fixtures, no explicit any in the test tree walkers. Verified: tsc --noEmit clean; muya lint clean; full muya suite 1508 tests pass (fold suites 55, incl. the new nested-fold regression + hiddenByFoldedDescendant unit tests). * test(muya): extract shared bootMuya harness to cut duplication (Sonar) SonarCloud quality gate flagged new-code duplicated-lines at 5.4% (threshold 3%), concentrated in the fold test files that each re-declared the same bootMuya + MUYA_VERSION setup/teardown. Extract that boilerplate into src/__tests__/muyaHarness.ts (useMuyaHarness + bootMuya) and consume it from headingFold.spec.ts and foldActions.spec.ts. This removes the duplicated setup blocks (the real duplication) rather than gaming the metric, and future block/UI specs can reuse it. Verified: muya tsc clean; lint clean; full muya suite 1508 tests pass (fold suites 55). * fix(muya): keep fold marker out of heading textContent (desktop e2e) The desktop E2E TOC tests (toc-scroll, toc-panel-content) read a heading via `heading.textContent` and match it against the heading text. The collapsed marker set `domNode.textContent = "…"`, so a folded heading read as "…Heading" and headingIndexByText() returned -1 — failing the TOC scroll/outline suites. This was a real regression from the marker, not pre-existing (thanks to the e2e log that pinpointed it). - Render the "…" glyph via CSS `.mu-fold-marker::after` instead of element text. The element stays a clickable, focusable button, but contributes no text to the heading, so `textContent` stays equal to the heading text. - Add a muya regression test asserting a folded heading textContent (normalized like the desktop consumer) is unchanged — the coverage the unit suite lacked. Verified: muya tsc clean; lint clean; full suite 1509 tests pass. * docs(muya): remove design docs from PR, keep rationale in code comments * refactor(muya): limit heading fold to per-heading toggle Address review feedback: drop the document-wide fold actions (Fold all / Unfold all / Fold to this level) and the clickable "..." collapsed-section marker. Keep the core per-heading chevron fold/unfold. Removes the now-dead production code (foldAll/unfoldAll/foldToLevel, the HeadingFoldMarker block + its CSS, the foldPlanForLevel helper, the front-menu fold items), plus the tests and locale strings that only covered the removed surface. * fix(muya): add missing fold toggle keys to ru locale ru was missing 'Fold this section' / 'Unfold this section' since the fold feature first landed, which fails localeCompleteness.spec.ts (every locale must carry the same keys as en). Unrelated to the scope reduction, but required to keep the suite green. * fix: integrate heading folding with editing and navigation * test: fix cross-platform fold navigation and undo scroll checks Use Home/End on Windows and Linux instead of word-navigation shortcuts. Assert the destination caret before typing and compare stale table picks against the post-undo scroll position. * fix: place heading fold toggle before the copy-link control * fix: align heading controls after the front menu button * fix: place heading copy-link before the front menu button * style: tighten spacing between heading controls * fix: use heading fold state for trailing arrow navigation * refactor: derive fold visibility from state and integrate block navigation * refactor: clarify heading types and simplify fold guards * fix: keep heading control listeners owned by their DOM nodes --------- Co-authored-by: Jocs <luoran1988@126.com>
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
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.
Closes #1869 Option to fold/collapse text
#1869
Summary
Adds heading section folding controls to both WYSIWYG and source-code editor modes.
In WYSIWYG mode, heading chevrons are shown when the heading is active or hovered, and remain visible when a section is folded.
In source-code mode, the fold gutter is moved to the left of the line numbers, uses matching chevron icons, follows the line-number color from the active theme, and applies the same active/hover/folded visibility behavior.
Type of change
Test plan
Added a new Playwright e2e spec covering heading fold chevron visibility in both WYSIWYG and source mode, including focus, hover, and gutter-hover behavior.
Validated with:
pnpm -C packages/desktop run buildpnpm -C packages/desktop exec playwright test test/e2e/heading-fold-chevrons.spec.tspnpm -C packages/desktop exec playwright test test/e2eResult: 75 passed, 2 skipped.
Notes for reviewers [optional]
The source editor uses CodeMirror 5 fold gutters. The chevron color is synchronized from the computed line-number color so it follows theme changes instead of duplicating theme color values.
Fold chevrons are intentionally hidden by default on unfolded headings, shown on active or hovered heading lines, and kept visible for folded headings to indicate hidden content.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.