Skip to content

feat(RichTextEsheet): document modes, shared MDY engine, media capture (#758) - #762

Open
garrity-miepub wants to merge 9 commits into
mainfrom
feat/758-richtextesheet-document-modes
Open

garrity-miepub wants to merge 9 commits into
mainfrom
feat/758-richtextesheet-document-modes

Conversation

@garrity-miepub

@garrity-miepub garrity-miepub commented Nov 12, 2026 •

Copy link
Copy Markdown
Collaborator
pr762.mov

What & why

Implements the core of #758: RichTextEsheet becomes the general-purpose MDY document experience, and EncounterVisit now builds on its primitives instead of owning them.

Phase 1 — shared MDY engine

  • New RichTextEsheet/documentMdy.ts: a generic createMdyDocumentEngine factory (parse / create / render / projection refresh / document-body plumbing).
  • EncounterVisit/mdy.ts is now a thin config over the factory — its public API and all 56 of its tests are unchanged.
  • RichTextEsheet gains its own full MDY round-trip (mdy.ts, model.ts): saved documents reopen via the new initialMdy prop, with parties recovered from front matter and definition optional when the MDY carries one.

Phase 2 — document modes

  • RichTextEsheet gets the four-mode experience previously exclusive to EncounterVisit: eSheet / RichEdit / View / Export, with the mode switcher riding the always-mounted narrative toolbar, the linked-section resolver dialog (focus-trapped), defaultMode, and setMode on the handle.
  • The document surfaces moved into RichTextEsheet (git mv, history preserved) as DocumentMdyEditor / DocumentMdyView / DocumentExportView / createMdyReportHtml; the encounter-named files remain as thin wrappers with visit wording. Shared chrome CSS re-scoped to :is(.encounter-visit, .richtext-esheet).

Media capture (generalized from #736)

  • useDocumentMediaCapture generalizes the encounter hook; EncounterVisit delegates with its clinical PHI wording.
  • New EsheetNarrativeMediaContext hands capture to the globally-registered narrative fields; in RichTextEsheet each field's capture bar starts hidden behind the block (+) menu (new TextAreaRichKerebron.defaultCaptureBarVisible, default true so existing hosts are unchanged).
  • Known contract (inherited from RichMediaCaptureEditor 4/4: EncounterVisit opt-in media capture (gated sanitizer relaxation) #736): field-captured images live in response attributes and MDY front matter, not the plain-text View/Export projections.

Evidence

  • 2733/2733 unit tests pass (pnpm test), including the unchanged EncounterVisit suites (405 tests) — behavior parity for the clinical component.
  • Browser-verified in Storybook: all four modes round-trip one draft, resolver dialog, macro insert → projection refresh, export front matter, per-field capture docks with confirm dialog.
  • New stories: Referral Letter / Support Ticket / Live MDY output / With Media Capture.

Remaining work on this PR

  • RichTextEsheet mode-switching component tests (EncounterVisitModes-style)
  • Mode-exercising story coverage beyond the defaults
  • MAINTAINERS.md updates (both components) + catalog guidance crosslinks

Checklist

  • pnpm typecheck && pnpm lint && pnpm format && pnpm test pass
  • Visual tests pass / baselines updated if rendering changed (pnpm test:visual)

RTL — see CONTRIBUTING.md → RTL & direction-agnostic styling

  • No new physical-direction classes or inline styles (ml-, left-, marginLeft, left:, textAlign: 'left', …) — pnpm rtl:scan passes
  • Directional icons flip with rtl:-scale-x-100; genuinely physical usages annotated // rtl-ignore -- <reason> (no new directional icons)
  • ArrowLeft/ArrowRight handlers are direction-aware (useDirection()) (no new arrow-key handlers)
  • New class strings safelisted in both src/tailwind-preset.ts and src/tailwind-preset.cjs (no new class strings; chrome classes reused from EncounterVisit)
  • Direction-sensitive rendering has RTL visual coverage: screenshot plus computed-style/geometry assertions

Closes #758

RichTextEsheet becomes the general-purpose MDY document experience and
EncounterVisit now builds on its primitives (#758):

- Extract the MDY document engine (parse/create/render/projection
  refresh) into RichTextEsheet/documentMdy.ts; EncounterVisit/mdy.ts is
  now a thin config over the shared factory with an unchanged public
  API, and RichTextEsheet gains its own full MDY round-trip (mdy.ts,
  model.ts) so saved documents reopen from initialMdy.
- Port the four-mode experience (eSheet, RichEdit, View, Export) into
  RichTextEsheet: mode switcher riding the always-mounted narrative
  toolbar, linked-section resolver dialog with focus trap, and new
  initialMdy/defaultMode props plus setMode on the handle; definition
  is now optional when the MDY front matter carries one.
- Move the document surfaces into RichTextEsheet as DocumentMdyEditor,
  DocumentMdyView, DocumentExportView and createMdyReportHtml (git mv,
  history preserved); the encounter-named files remain as thin wrappers
  with visit wording, so EncounterVisit behavior and tests are
  unchanged.
- Opt-in media capture: useDocumentMediaCapture generalizes the #736
  hook (EncounterVisit delegates with its clinical PHI wording), and
  the new EsheetNarrativeMediaContext hands capture to the globally
  registered narrative fields. In RichTextEsheet each field's capture
  bar starts hidden behind the block (+) menu via the new
  TextAreaRichKerebron defaultCaptureBarVisible prop.
- Scope the shared document chrome CSS from .encounter-visit to
  :is(.encounter-visit, .richtext-esheet).

Refs #758
@garrity-miepub
garrity-miepub marked this pull request as ready for review November 12, 2026 23:18
Copilot AI balanced review requested due to automatic review settings November 12, 2026 23:18
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Deploying ui with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0260d97
Status: ✅  Deploy successful!
Preview URL: https://e2782fc8.ui-6d0.pages.dev
Branch Preview URL: https://feat-758-richtextesheet-docu.ui-6d0.pages.dev

View logs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical issues can submit after read-only changes, overwrite newly added section content with stale rich-text data, and accept native imports belonging to another document.

5 open issues
What changed in this PR

Expands RichTextEsheet into a general-purpose MDY document editor and moves reusable document primitives out of EncounterVisit, advancing the shared architecture described in #758.

Changes:

  • Adds eSheet, RichEdit, View, and Export modes, linked-section editing, and MDY reopening.
  • Extracts shared serialization, rendering, export, and media-capture functionality while retaining encounter-specific wrappers.
  • Adds field-level capture controls and MDY round-trip tests.
File Description
src/​styles/​encounter-visit.css Shares document chrome styling between both components.
src/​esheet.ts Exports narrative media context and types.
src/​esheet-fields/​EsheetNarrativeField.tsx Accepts capture configuration through context.
src/​components/​TextAreaRich/​TextAreaRichKerebron.tsx Adds initial capture-bar visibility control.
src/​components/​RichTextEsheet/​RichTextEsheet.tsx Adds modes, reopening, resolver, and capture integration.
src/​components/​RichTextEsheet/​RichTextEsheet.test.tsx Updates the expected linked MDY output.
src/​components/​RichTextEsheet/​RichTextEsheet.stories.tsx Adds a media-capture example.
src/​components/​RichTextEsheet/​report.ts Houses shared sanitized report generation.
src/​components/​RichTextEsheet/​model.ts Derives snapshots with portable MDY.
src/​components/​RichTextEsheet/​model.test.ts Tests snapshots and MDY round trips.
src/​components/​RichTextEsheet/​mediaCapture.tsx Provides shared capture configuration and confirmation.
src/​components/​RichTextEsheet/​mdy.ts Configures generic document serialization and projections.
src/​components/​RichTextEsheet/​index.ts Exports new document APIs and types.
src/​components/​RichTextEsheet/​DocumentMdyView.tsx Provides the shared document preview.
src/​components/​RichTextEsheet/​DocumentMdyEditor.tsx Provides the shared linked-field rich editor.
src/​components/​RichTextEsheet/​documentMdy.ts Implements the configurable shared MDY engine.
src/​components/​RichTextEsheet/​DocumentExportView.tsx Provides shared source display and download.
src/​components/​RichTextEsheet/​definition.ts Adds mode types and separates snapshot generation.
src/​components/​RichTextEsheet/​definition.test.ts Moves snapshot tests to the model suite.
src/​components/​EncounterVisit/​report.ts Re-exports shared report generation.
src/​components/​EncounterVisit/​mediaCapture.tsx Delegates capture with clinical warning text.
src/​components/​EncounterVisit/​mdy.ts Configures the shared engine for encounter documents.
src/​components/​EncounterVisit/​EncounterMdyView.tsx Wraps preview with encounter wording.
src/​components/​EncounterVisit/​EncounterMdyEditor.tsx Wraps the shared editor with its encounter label.
src/​components/​EncounterVisit/​EncounterExportView.tsx Wraps export with encounter wording.

Review effort: Balanced

…iner docs (#758)

- RichTextEsheetModes.test.tsx: 6 tests covering definition/id guards,
  rich-edit flush on mode switch and save, Export download, focusSection
  from RichEdit, the linked-section resolver dialog, and read-only
  restore from portable MDY.
- Stories: RichEdit, FinalView, and Restored from MDY (read-only,
  definition + parties from front matter). Component docs drop the stale
  "MDY is export-only" limitation and document the four modes and
  initialMdy round-trip.
- MAINTAINERS: RichTextEsheet gains shared-engine/modes/media-capture
  sections; EncounterVisit notes the one-way delegation to
  RichTextEsheet. Catalog Editors row mentions modes + MDY round-trip.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 23:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Missing native form identity validation allows one document’s answers to load into another and be silently rewritten under the wrong document identity.

7 open issues
1 previously missed issue

In code that hasn't changed since last review

Low severity Add browser coverage for resolver modes and toolbar layouts

src/​components/​RichTextEsheet/​RichTextEsheet.tsx:710

tests/visual/richtext-esheet.spec.ts never switches modes, leaving the new protected-link resolver and expanded toolbar without browser regression coverage in this host. Add real-editor mode round-trip checks and mobile/desktop and RTL screenshots that assert toolbar and resolver geometry, as required for rendering changes by CONTRIBUTING.md:136–137,257–263.

Review effort: Balanced

- Reject a native eSheet MDY whose form id does not match the supplied
  definition (parity with EncounterVisit's guard).
- Derive the RichEdit fallback body from the live snapshot instead of a
  mount-time seed, so a configuration change cannot be erased by the
  first flush. Same fix mirrored in EncounterVisit.
- Recheck onSubmit/readOnly/saving after submit's await of the editor
  flush (parity with EncounterVisit).
- Treat an open alertdialog (screen-capture confirmation) as in scope
  for the resolver focus trap so keyboard users can confirm or cancel.
  Same fix mirrored in EncounterVisit's trap.
- Tests: form-id guard, fallback-body regeneration after a config
  change, rejected-save draft retention, and capture-confirmation focus
  inside the resolver trap.
Copilot AI balanced review requested due to automatic review settings November 12, 2026 23:51
…boundary (#762 review)

- MAINTAINERS: the front matter is the canonical store (mdy metadata,
  native form, response map, full definition, parties, flat projections);
  the generated body links every section — only View prunes empties —
  and multi-line answers join with <br>, not paragraphs.
- WithMediaCapture story: field-captured images persist in response
  attributes and MDY front matter but do not render in RichEdit, View or
  Export's plain-answer body projections.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Linked-section focus fails with custom host renderers, Escape incorrectly dismisses the resolver behind child modals, and media capture lacks provider-only regression coverage and documentation of screenshot projection limitations.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

Medium severity Prevent Escape from closing the resolver behind child modals

src/​components/​RichTextEsheet/​RichTextEsheet.tsx:436

Escape is handled globally even when a nested modal owns the interaction. With media capture enabled, opening “Annotate image” inside the linked-section resolver and pressing Escape invokes both the annotation modal's listener and this listener, closing the resolver as well and hiding its section instead of returning to section editing. Ignore Escape while a child modal is present so only the topmost dialog handles it; this also prevents Escape behind the screen-capture confirmation from dismissing the resolver.

Medium severity Mark linked sections independently of renderer markup

src/​components/​RichTextEsheet/​RichTextEsheet.tsx:529

The section registry preserves host-registered section renderers, but those renderers need not emit the private .section-field-preview class. For such a host, this query finds no section to mark, and the resolver's .field-wrapper:not([data-encounter-focus]) rule hides the requested section along with all the others. Mark the native section wrapper using its generated data-field-id, independently of the section renderer's markup.

Medium severity Add provider-only media sanitizer regression tests

src/​esheet-fields/​EsheetNarrativeField.tsx:305

This adds a context-based path for enabling the media sanitizer, but the existing narrative media tests supply only explicit props and the mode tests stub narrative capture. Add provider-only regression coverage for uploaded-image round trips, rejection without an upload handler, explicit-prop precedence, and forwarding the hidden-bar default. These tests protect the opt-in sanitizer boundary used by globally registered fields.

7 resolved since last review

Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings November 13, 2026 00:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because Escape can close the resolver behind an open confirmation dialog, creating a focus-management conflict.

No open issues 🎉

Review effort: Balanced

…ctor

The SupportTicket story now renders the Document mode selector bar
(this branch's feature), shifting the phone layout ~6% past the 5%
diff threshold — the stale baseline made the Visual job flap across
commits. Regenerated against the static CI-parity build; desktop and
RTL baselines are unaffected (ReferralLetter has no mode selector).
Copilot AI balanced review requested due to automatic review settings November 13, 2026 01:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Active issues leave the page scroll-locked after nested-dialog cancellation and expose stale, editable answers for removed sections instead of matching View/Export behavior.

2 open issues

Review effort: Balanced

In CI the paragraph click can land before ProseMirror is interactive,
leaving the caret in the heading — headings drop images in markdown
serialization, so the uploadHandler round-trip failed through all
retries on slow runners (this branch's chunk layout shifted the story's
mount timing). Retry the click until the selection really sits in the
target paragraph, in both media-capture round-trip tests.
Copilot AI balanced review requested due to automatic review settings November 13, 2026 01:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Pressing Escape in the screen-capture confirmation closes the underlying resolver, leaving the page scroll-locked when the confirmation subsequently closes.

2 open issues

Review effort: Balanced

Copilot review follow-ups on #762:

- A same-id definition that drops a section left its imported link in
  RichEdit showing the stale answer, editable and unprotected, while
  View/Export already normalized it. The engine now exposes
  retiredFields() (shared with create()'s retirement pass), and
  RichTextEsheet feeds retired projections to DocumentMdyEditor so they
  render 'Field no longer configured', stay edit-protected, and ignore
  activation — preserving authored prose, caret, and undo.

- Escape during the screen-capture confirmation closed the resolver
  underneath it (AlertDialog ignores Escape by design), desyncing
  Modal's scroll lock and leaving the page unscrollable. The resolver's
  Escape handler now defers while an alertdialog is open, matching the
  focus trap's existing scope exception.
Copilot AI balanced review requested due to automatic review settings November 13, 2026 01:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because unresolved edge cases can leave stale linked content, mishandle nested dialogs, and bypass configured media-upload restrictions.

No open issues 🎉

2 resolved since last review

Review effort: Balanced

The DocumentPreview story's scrollable <pre> failed axe's
scrollable-region-focusable on Linux CI (overlay scrollbars hide it on
macOS). Apply the catalog's standard remedy: role=region + aria-label +
tabIndex=0 (same pattern as the EmailEditor MJML output).
Copilot AI balanced review requested due to automatic review settings November 13, 2026 02:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed because cross-editor updates can overwrite unpublished prose, alongside broader media-handling, accessibility, and shared-orchestration concerns.

No open issues 🎉

Review effort: Balanced

The existing RTL test uses ReferralLetter, which never renders the new
Document mode selector. Cover SupportTicket under direction:rtl with a
screenshot plus geometry assertions: shell direction is rtl and the
switcher's sticky overlay (justify-start + ms-1) docks at the inline
start — the right edge in RTL.
Copilot AI balanced review requested due to automatic review settings November 13, 2026 02:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved issues leave removed-section answers visible and unprotected, trigger full-document reloads during typing, and allow Escape to dismiss the linked-section resolver during an annotation upload.

3 open issues

Review effort: Balanced

Comment on lines +257 to +261
const richBody = React.useMemo(
() =>
getRichTextEsheetDocumentBody(snapshot.responses) ??
parseRichTextEsheetMdy(snapshot.mdy).body,
[snapshot]
Comment on lines +310 to +312
const next = updateRichTextEsheetDocumentBody(current, body, {
_ai: false,
});
// Escape by design — the same scope exception as the focus trap below.
// Closing the resolver beneath it would also desync Modal's scroll
// lock and leave the page unscrollable after the confirmation closes.
if (document.querySelector('[role="alertdialog"]')) return;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make EncounterVisit a minimal RichTextEsheet wrapper with related document creation

2 participants