Skip to content

Commit b1765cc

Browse files
makhnatkinmakhnatkin
committed
fix(marks): apply mark to whole selection when not fully covered
1 parent 61360f0 commit b1765cc

7 files changed

Lines changed: 186 additions & 22 deletions

File tree

‎packages/editor/src/bundle/toolbar/markup/MToolbarColors.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export const MToolbarColors: React.FC<MToolbarColorsProps> = ({
1010
focus,
1111
onClick,
1212
}) => {
13+
// TODO: @makhnatkin check markup mode
1314
return (
1415
<ToolbarColors
1516
enable

‎packages/editor/src/bundle/toolbar/wysiwyg/WToolbarColors.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export const WToolbarColors: React.FC<WToolbarColorsProps> = ({
2727
enable={enabled}
2828
currentColor={currentColor}
2929
exec={(color) => {
30-
action.run({color: color === currentColor ? '' : color});
30+
action.run({color});
3131
}}
3232
disablePortal={disablePortal}
3333
className={className}

‎packages/editor/src/extensions/yfm/Color/index.ts‎

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import {toggleMark} from 'prosemirror-commands';
2+
import {TextSelection} from 'prosemirror-state';
23

34
import type {Action, ExtensionAuto} from '../../../core';
4-
import {isMarkActive} from '../../../utils/marks';
5+
import {selectionAllHasMarkWithAttr} from '../../../utils/marks';
56

67
import {ColorSpecs, colorType} from './ColorSpecs';
78
import {type Colors, colorAction, colorMarkName} from './const';
8-
import {chainAND, parseStyleColorValue, validateClassNameColorName} from './utils';
9+
import {parseStyleColorValue, validateClassNameColorName} from './utils';
910

1011
import './colors.scss';
1112

@@ -25,29 +26,63 @@ export const Color: ExtensionAuto = (builder) => {
2526
builder.addAction(colorAction, ({schema}) => {
2627
const type = colorType(schema);
2728
return {
28-
isActive: (state) => Boolean(isMarkActive(state, type)),
29+
isActive: (state) =>
30+
Boolean(type.isInSet(state.storedMarks ?? state.selection.$to.marks())),
2931
isEnable: toggleMark(type),
3032
run: (state, dispatch, _view, attrs) => {
3133
const params = attrs as ColorActionParams | undefined;
32-
const hasMark = isMarkActive(state, type);
34+
const color = params?.[colorMarkName];
3335

34-
if (!params || !params[colorMarkName]) {
35-
if (!hasMark) return true;
36+
if (dispatch) {
37+
const {empty, $cursor} = state.selection as TextSelection;
3638

37-
// remove mark
38-
return toggleMark(type, params)(state, dispatch);
39-
}
39+
if (empty && $cursor) {
40+
// cursor only — toggle stored marks
41+
const storedMark = type.isInSet(state.storedMarks ?? $cursor.marks());
42+
if (!color || storedMark?.attrs[colorMarkName] === color) {
43+
dispatch(state.tr.removeStoredMark(type));
44+
} else {
45+
dispatch(
46+
state.tr.addStoredMark(type.create({[colorMarkName]: color})),
47+
);
48+
}
49+
return true;
50+
}
4051

41-
if (hasMark) {
42-
// remove old mark, then add new with new color
43-
return chainAND(toggleMark(type), toggleMark(type, params))(state, dispatch);
52+
const tr = state.tr;
53+
if (!color) {
54+
// "default" / remove color: always strip
55+
state.selection.ranges.forEach(({$from, $to}) =>
56+
tr.removeMark($from.pos, $to.pos, type),
57+
);
58+
} else {
59+
const allSameColor = selectionAllHasMarkWithAttr(
60+
state,
61+
type,
62+
colorMarkName,
63+
color,
64+
);
65+
state.selection.ranges.forEach(({$from, $to}) => {
66+
if (allSameColor) {
67+
tr.removeMark($from.pos, $to.pos, type);
68+
} else {
69+
// addMark replaces any existing color mark (same type = mutually exclusive)
70+
tr.addMark(
71+
$from.pos,
72+
$to.pos,
73+
type.create({[colorMarkName]: color}),
74+
);
75+
}
76+
});
77+
}
78+
dispatch(tr.scrollIntoView());
4479
}
45-
46-
// add mark
47-
return toggleMark(type, params)(state, dispatch);
80+
return true;
4881
},
4982
meta(state): Colors {
50-
return type.isInSet(state.selection.$to.marks())?.attrs[colorMarkName];
83+
return type.isInSet(state.storedMarks ?? state.selection.$to.marks())?.attrs[
84+
colorMarkName
85+
];
5186
},
5287
};
5388
});

‎packages/editor/src/markup/commands/marks.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import {inlineWrapTo, toggleInlineMarkupFactory} from './helpers';
1+
import {toggleInlineMarkupFactory} from './helpers';
22

3-
export const colorify = (color: string) => inlineWrapTo(`{${color}}(`, ')');
3+
export const colorify = (color: string) =>
4+
toggleInlineMarkupFactory({before: `{${color}}(`, after: ')'});
45

56
export const toggleBold = toggleInlineMarkupFactory('**');
67
export const toggleItalic = toggleInlineMarkupFactory('_');

‎packages/editor/src/utils/actions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export function defineActions<Keys extends string>(actions: Record<Keys, ActionS
1010
}
1111

1212
export function createToggleMarkAction(markType: MarkType): ActionSpec {
13-
const command = toggleMark(markType);
13+
const command = toggleMark(markType, undefined, {removeWhenPresent: false});
1414
return {
1515
isActive: (state) => Boolean(isMarkActive(state, markType)),
1616
isEnable: command,

‎packages/editor/src/utils/marks.test.ts‎

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {EditorState, TextSelection} from 'prosemirror-state';
55
import type {Parser} from '../core/types/parser';
66
import {ParserFacet} from '../core/utils/parser';
77

8-
import {canApplyInlineMarkInMarkdown} from './marks';
8+
import {canApplyInlineMarkInMarkdown, selectionAllHasMarkWithAttr} from './marks';
99

1010
const schema = new Schema({
1111
nodes: {
@@ -15,6 +15,21 @@ const schema = new Schema({
1515
},
1616
});
1717

18+
// Schema with a color mark (parameterised, excludes itself)
19+
const colorSchema = new Schema({
20+
nodes: {
21+
doc: {content: 'block+'},
22+
paragraph: {content: 'inline*', group: 'block', marks: '_'},
23+
text: {group: 'inline'},
24+
},
25+
marks: {
26+
color: {
27+
attrs: {color: {}},
28+
excludes: '_',
29+
},
30+
},
31+
});
32+
1833
const md = new MarkdownIt();
1934
const mockParser: Parser = {
2035
isPunctChar: (ch: string) => md.utils.isPunctChar(ch),
@@ -37,6 +52,87 @@ function canApply(text: string, from: number, to: number): boolean {
3752
return canApplyInlineMarkInMarkdown(state);
3853
}
3954

55+
// ─── helpers for selectionAllHasMarkWithAttr tests ───────────────────────────
56+
57+
const colorMark = colorSchema.marks.color;
58+
59+
/**
60+
* Build a state whose paragraph contains segments described by `parts`.
61+
* Each part is either a plain string, or {text, color} for a colored segment.
62+
* `from`/`to` are 0-based character indices inside the paragraph text.
63+
*/
64+
function makeColorState(
65+
parts: Array<string | {text: string; color: string}>,
66+
from: number,
67+
to: number,
68+
): EditorState {
69+
const nodes = parts.map((p) => {
70+
if (typeof p === 'string') return colorSchema.text(p);
71+
return colorSchema.text(p.text, [colorMark.create({color: p.color})]);
72+
});
73+
const doc = colorSchema.node('doc', null, [colorSchema.node('paragraph', null, nodes)]);
74+
// PM positions: 0=before doc, 1=start of paragraph content
75+
const sel = TextSelection.create(doc, from + 1, to + 1);
76+
return EditorState.create({doc, selection: sel});
77+
}
78+
79+
function allHasColor(
80+
parts: Array<string | {text: string; color: string}>,
81+
from: number,
82+
to: number,
83+
color: string,
84+
): boolean {
85+
const state = makeColorState(parts, from, to);
86+
return selectionAllHasMarkWithAttr(state, colorMark, 'color', color);
87+
}
88+
89+
describe('selectionAllHasMarkWithAttr', () => {
90+
it('returns true when entire selection has the exact color', () => {
91+
// "ABC" all red — select all 3 chars
92+
expect(allHasColor([{text: 'ABC', color: 'red'}], 0, 3, 'red')).toBe(true);
93+
});
94+
95+
it('returns false when part of the selection has no color', () => {
96+
// "AB" red, "C" plain — select all 3
97+
expect(allHasColor([{text: 'AB', color: 'red'}, 'C'], 0, 3, 'red')).toBe(false);
98+
});
99+
100+
it('returns false when part of the selection has a different color', () => {
101+
// "AB" red, "C" blue — select all 3, check for red
102+
expect(
103+
allHasColor(
104+
[
105+
{text: 'AB', color: 'red'},
106+
{text: 'C', color: 'blue'},
107+
],
108+
0,
109+
3,
110+
'red',
111+
),
112+
).toBe(false);
113+
});
114+
115+
it('returns false when checking a color that is not applied', () => {
116+
// "ABC" all red — check for blue
117+
expect(allHasColor([{text: 'ABC', color: 'red'}], 0, 3, 'blue')).toBe(false);
118+
});
119+
120+
it('returns true when selection covers only a whitespace-only node (skipped)', () => {
121+
// " " plain spaces — whitespace-only nodes are skipped, so result is vacuously true
122+
expect(allHasColor([' '], 0, 3, 'red')).toBe(true);
123+
});
124+
125+
it('returns true for sub-selection that is entirely colored', () => {
126+
// "A" plain, "BCD" red, "E" plain — select chars 1–4 (BCD)
127+
expect(allHasColor(['A', {text: 'BCD', color: 'red'}, 'E'], 1, 4, 'red')).toBe(true);
128+
});
129+
130+
it('returns false for sub-selection that spans colored and plain', () => {
131+
// "AB" plain, "CD" red — select chars 1–4 (BCD)
132+
expect(allHasColor(['AB', {text: 'CD', color: 'red'}], 1, 4, 'red')).toBe(false);
133+
});
134+
});
135+
40136
describe('canApplyInlineMarkInMarkdown', () => {
41137
it('allows empty selection (cursor)', () => {
42138
expect(canApply('hello,', 0, 0)).toBe(true);

‎packages/editor/src/utils/marks.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type {Mark, MarkType, Node} from 'prosemirror-model';
1+
import type {Attrs, Mark, MarkType, Node} from 'prosemirror-model';
22
import type {EditorState} from 'prosemirror-state';
33

44
import {getParserFromState} from '../core/utils/parser';
@@ -17,6 +17,37 @@ export function isMarkActive(state: EditorState, type: MarkType) {
1717
return state.doc.rangeHasMark(from, to, type);
1818
}
1919

20+
/**
21+
* Returns `true` if every non-whitespace text node in the selection has the given mark type
22+
* with the given attr key set to exactly `attrValue`.
23+
*
24+
* Used to decide whether applying a parameterised mark (e.g. color) should toggle it off
25+
* (full coverage with the same value) or apply it to the whole selection.
26+
*/
27+
export function selectionAllHasMarkWithAttr(
28+
state: EditorState,
29+
markType: MarkType,
30+
attrKey: string,
31+
attrValue: Attrs[string],
32+
): boolean {
33+
return state.selection.ranges.every((r) => {
34+
let allHave = true;
35+
state.doc.nodesBetween(r.$from.pos, r.$to.pos, (node, _pos, parent) => {
36+
if (!allHave) return false;
37+
if (
38+
node.isText &&
39+
parent?.type.allowsMarkType(markType) &&
40+
!/^\s*$/.test(node.text!)
41+
) {
42+
const mark = markType.isInSet(node.marks);
43+
allHave = Boolean(mark) && mark!.attrs[attrKey] === attrValue;
44+
}
45+
return undefined;
46+
});
47+
return allHave;
48+
});
49+
}
50+
2051
/**
2152
* Returns `false` when the current selection cannot be wrapped in an inline mark
2253
* without breaking markdown round-trip, per CommonMark flanking delimiter rules:

0 commit comments

Comments
 (0)