Skip to content

Commit 4c04ebd

Browse files
committed
fix: address PR review — minProperties, RichContentInsertInput type, deduplicate alignment constant
1 parent 1e4e294 commit 4c04ebd

4 files changed

Lines changed: 50 additions & 30 deletions

File tree

packages/document-api/src/contract/schemas.ts

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4869,24 +4869,27 @@ const operationSchemas: Record<OperationId, OperationSchemaSet> = {
48694869
id: { type: 'string' },
48704870
op: { const: 'format.apply', type: 'string' },
48714871
where: stepWhereSchema,
4872-
args: objectSchema(
4873-
{
4874-
inline: buildInlineRunPatchSchema(),
4875-
alignment: {
4876-
type: 'string',
4877-
enum: ['left', 'center', 'right', 'justify'],
4878-
description:
4879-
'Set paragraph alignment on the target block(s). Can be combined with inline formatting in the same step.',
4880-
},
4881-
scope: {
4882-
type: 'string',
4883-
enum: ['match', 'block'],
4884-
description:
4885-
'When "block", inline formatting expands to cover the entire parent paragraph(s), not just the matched text. Use "block" after markdown inserts to format whole paragraphs with a short identifying pattern. Default: "match".',
4872+
args: {
4873+
...objectSchema(
4874+
{
4875+
inline: buildInlineRunPatchSchema(),
4876+
alignment: {
4877+
type: 'string',
4878+
enum: ['left', 'center', 'right', 'justify'],
4879+
description:
4880+
'Set paragraph alignment on the target block(s). Can be combined with inline formatting in the same step.',
4881+
},
4882+
scope: {
4883+
type: 'string',
4884+
enum: ['match', 'block'],
4885+
description:
4886+
'When "block", inline formatting expands to cover the entire parent paragraph(s), not just the matched text. Use "block" after markdown inserts to format whole paragraphs with a short identifying pattern. Default: "match".',
4887+
},
48864888
},
4887-
},
4888-
[], // No individual field is required — at least one must be present
4889-
),
4889+
[], // No individual field is required
4890+
),
4891+
minProperties: 1, // At least one of inline, alignment, or scope must be present
4892+
},
48904893
},
48914894
['id', 'op', 'where', 'args'],
48924895
);

packages/document-api/src/insert/insert.ts

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import type { SelectionTarget, TargetLocator, SDMutationReceipt } from '../types
33
import type { SDInsertInput } from '../types/structural-input.js';
44
import type { SDFragment } from '../types/fragment.js';
55
import type { StoryLocator } from '../types/story.types.js';
6+
import type { BlockNodeAddress } from '../types/base.js';
7+
import type { Placement } from '../types/placement.js';
68
import { PLACEMENT_VALUES } from '../types/placement.js';
79
import { DocumentApiValidationError } from '../errors.js';
810
import {
@@ -40,6 +42,25 @@ export type TextInsertInput = OptionalInsertLocator & {
4042
in?: StoryLocator;
4143
};
4244

45+
/**
46+
* Extended input for markdown/html inserts that also accept BlockNodeAddress
47+
* targets and placement. These route through the structural insert path.
48+
*/
49+
export type RichContentInsertInput = {
50+
/** Block target for positioned inserts. Accepts BlockNodeAddress or SelectionTarget. */
51+
target?: SelectionTarget | BlockNodeAddress;
52+
/** Optional mutation ref. Mutually exclusive with target. */
53+
ref?: string;
54+
/** The markdown/html content to insert. */
55+
value: string;
56+
/** Content format — must be 'markdown' or 'html' for this input shape. */
57+
type: 'markdown' | 'html';
58+
/** Where to place content relative to target. Only valid with BlockNodeAddress targets. */
59+
placement?: Placement;
60+
/** Target a specific document story. */
61+
in?: StoryLocator;
62+
};
63+
4364
/** @deprecated Use {@link TextInsertInput} instead. */
4465
export type LegacyInsertInput = TextInsertInput;
4566

@@ -53,7 +74,7 @@ export type LegacyInsertInput = TextInsertInput;
5374
* Discrimination: presence of `content` (structural) vs `value` (text string).
5475
* These are mutually exclusive — providing both is an error.
5576
*/
56-
export type InsertInput = TextInsertInput | SDInsertInput;
77+
export type InsertInput = TextInsertInput | RichContentInsertInput | SDInsertInput;
5778

5879
// ---------------------------------------------------------------------------
5980
// Allowlists for strict field validation
@@ -276,16 +297,19 @@ export function executeInsert(
276297
}
277298

278299
// Text string path
279-
const { target, ref, value } = input;
280300
const contentType = input.type ?? 'text';
281301

282302
// For non-text content types, delegate to the adapter's structured insert path.
283303
if (contentType !== 'text') {
284304
return writeAdapter.insertStructured(input, normalizeMutationOptions(options));
285305
}
286306

307+
// After the non-text branch, input is guaranteed to be a plain TextInsertInput.
308+
const textInput = input as TextInsertInput;
309+
const { target, ref, value } = textInput;
310+
287311
// Text path with target/ref → route through SelectionMutationAdapter
288-
const storyIn = input.in;
312+
const storyIn = textInput.in;
289313
if (target || ref) {
290314
const request = target
291315
? { kind: 'insert' as const, target, text: value, ...(storyIn ? { in: storyIn } : {}) }

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/executor.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import type {
3939
} from './executor-registry.types.js';
4040
import { getStepExecutor } from './executor-registry.js';
4141
import { planError } from './errors.js';
42+
import { ALIGNMENT_TO_JUSTIFICATION } from './paragraphs-wrappers.js';
4243
import { closeHistory } from 'prosemirror-history';
4344
import { yUndoPluginKey } from 'y-prosemirror';
4445
import { checkRevision, getRevision } from './revision-tracker.js';
@@ -804,21 +805,13 @@ export function executeTextDelete(
804805
return { changed: true };
805806
}
806807

807-
/** Alignment API value → OOXML justification value */
808-
const ALIGNMENT_TO_JUSTIFICATION: Record<string, string> = {
809-
left: 'left',
810-
center: 'center',
811-
right: 'right',
812-
justify: 'both',
813-
};
814-
815808
/**
816809
* Applies alignment to the paragraph node(s) that contain the given range.
817810
* Uses the same mechanism as paragraphsSetAlignmentWrapper: updates
818811
* paragraphProperties.justification via tr.setNodeMarkup.
819812
*/
820813
function applyAlignmentToRange(tr: Transaction, absFrom: number, absTo: number, alignment: string): boolean {
821-
const justification = ALIGNMENT_TO_JUSTIFICATION[alignment];
814+
const justification = ALIGNMENT_TO_JUSTIFICATION[alignment as keyof typeof ALIGNMENT_TO_JUSTIFICATION];
822815
if (!justification) return false;
823816

824817
let changed = false;

packages/super-editor/src/editors/v1/document-api-adapters/plan-engine/paragraphs-wrappers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ function mutateParagraphProperties(
238238
// Alignment mapping — external API → OOXML justification value
239239
// ---------------------------------------------------------------------------
240240

241-
const ALIGNMENT_TO_JUSTIFICATION: Record<ParagraphAlignment, string> = {
241+
export const ALIGNMENT_TO_JUSTIFICATION: Record<ParagraphAlignment, string> = {
242242
left: 'left',
243243
center: 'center',
244244
right: 'right',

0 commit comments

Comments
 (0)