Skip to content

Commit f2ef303

Browse files
JohnMcLearclaude
andcommitted
feat(toolbarSelect): DRY up <select>-change → ace edit → focus-restore
Three Etherpad plugins (ep_font_color, ep_font_size, ep_headings2) all implement the same toolbar-select boilerplate by hand: hs.on('change', function () { const value = $(this).val(); const intValue = parseInt(value, 10); if (!isNaN(intValue)) { context.ace.callWithAce((ace) => ace.ace_doX(intValue), 'op', true); hs.val('dummy'); context.ace.focus(); } }); This commit adds a shared `toolbarSelect({selector, context, invoke})` helper that handles all four steps — value coercion, callWithAce wrap, sentinel reset, and (most importantly for a11y) editor-focus restore so the next keystroke doesn't get swallowed by the toolbar control. Focus restoration runs unconditionally, even when the coerced value is unusable, so an accidental pick on a non-numeric option still leaves the user typing back in the pad rather than stuck on the select. This preserves the WCAG-relevant property that the per-plugin .focus() calls already provided — see ether/etherpad#7255 for the broader context. Client-only module — imported via the sub-path `ep_plugin_helpers/toolbar-select`, not via the package root, so esbuild doesn't pull any server-only deps into the pad bundle (same pattern as pad-toggle / pad-select). 12 new mocha tests cover config validation, int/number/string/identity and custom-function coercers, custom resetValue, onAfterChange callback invocation, and the focus-runs-on-failure guarantee. Bumps minor version 0.5.3 → 0.6.0 because it's a new public API surface; no behavior change for existing helpers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c0fe05d commit f2ef303

4 files changed

Lines changed: 402 additions & 1 deletion

File tree

README.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,59 @@ The plugin's `ep.json` must list each hook on the right side:
245245
- `enforceSettings` on → use the pad-wide value
246246
- `enforceSettings` off → use the user cookie value, falling back to pad-wide, falling back to `defaultEnabled`
247247

248+
### ToolbarSelect
249+
250+
DRY up the toolbar `<select>`-change → ace edit → focus-restore boilerplate that plugins like `ep_font_color`, `ep_font_size`, and `ep_headings2` each implement by hand:
251+
252+
```js
253+
// before — repeated in each plugin
254+
exports.postAceInit = (hookName, context) => {
255+
const hs = $('#font-size, select.size-selection');
256+
hs.on('change', function () {
257+
const value = $(this).val();
258+
const intValue = parseInt(value, 10);
259+
if (!isNaN(intValue)) {
260+
context.ace.callWithAce((ace) => {
261+
ace.ace_doInsertsizes(intValue);
262+
}, 'insertsize', true);
263+
hs.val('dummy');
264+
context.ace.focus();
265+
}
266+
});
267+
};
268+
```
269+
270+
With this helper:
271+
272+
```js
273+
// Client-only — import the sub-path directly so esbuild doesn't pull
274+
// any server-only deps into the pad bundle.
275+
const {toolbarSelect} = require('ep_plugin_helpers/toolbar-select');
276+
277+
exports.postAceInit = (hookName, context) => {
278+
toolbarSelect({
279+
selector: '#font-size, select.size-selection',
280+
context,
281+
invoke: (ace, value) => ace.ace_doInsertsizes(value),
282+
op: 'insertsize',
283+
});
284+
};
285+
```
286+
287+
**Config:**
288+
289+
| Field | Required | Default | Description |
290+
|-------|----------|---------|-------------|
291+
| `selector` | yes || jQuery selector for the toolbar `<select>`. |
292+
| `context` | yes || The `context` argument from `postAceInit` — must expose `context.ace.callWithAce` and `context.ace.focus`. |
293+
| `invoke` | yes || `(ace, coercedValue) => void`. Runs inside `callWithAce` so the edit joins the undo stack. |
294+
| `op` | no | `'toolbarSelect'` | `callWithAce` label — useful for debugging the undo stack. |
295+
| `coerce` | no | `'int'` | One of `'int' \| 'number' \| 'string' \| 'identity'`, or a custom `(raw) => coerced \| null`. Returning `null` skips the edit but still restores focus. |
296+
| `resetValue` | no | `'dummy'` | Value to write back to the select after a successful edit, so picking the same option again still fires `change`. |
297+
| `onAfterChange` | no || `(coercedValue) => void`, called after focus is restored. Errors are swallowed and logged so a buggy callback can't break the editor. |
298+
299+
Focus restoration runs unconditionally — even if `coerce` returned `null` — so an accidental pick on a non-numeric option leaves the user typing back in the pad, not on the toolbar control.
300+
248301
### Message Relay
249302

250303
Intercept and relay real-time COLLABROOM messages.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "ep_plugin_helpers",
3-
"version": "0.5.3",
3+
"version": "0.6.0",
44
"description": "Shared factory functions to eliminate boilerplate across Etherpad plugins",
55
"author": {
66
"name": "John McLear",

test/toolbar-select.js

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
const {toolbarSelect} = require('../toolbar-select');
5+
6+
// Minimal jQuery-shaped stub. The real plugin runs under Etherpad's bundled
7+
// jQuery; the helper only uses `.on('change', fn)` and `.val(...)`.
8+
const makeFakeSelect = (initialValue = '12') => {
9+
let value = initialValue;
10+
let changeHandler = null;
11+
const $el = {
12+
on(event, handler) {
13+
if (event !== 'change') throw new Error(`unexpected event: ${event}`);
14+
changeHandler = handler;
15+
return $el;
16+
},
17+
val(v) {
18+
if (v === undefined) return value;
19+
value = v;
20+
return $el;
21+
},
22+
// Test-only helper: simulate the user picking an option.
23+
_fire(newValue) {
24+
value = newValue;
25+
if (changeHandler) changeHandler.call({_isThis: true, _$: $el});
26+
},
27+
};
28+
return $el;
29+
};
30+
31+
const installJqueryStub = ($el) => {
32+
// Capture the previous global so multiple tests can swap stubs cleanly.
33+
const prev = globalThis.window;
34+
globalThis.window = {
35+
$: (selectorOrThis) => {
36+
// window.$(this) inside the change handler returns the element wrapper;
37+
// window.$(selector) returns the captured $el.
38+
if (selectorOrThis && selectorOrThis._isThis) return selectorOrThis._$;
39+
return $el;
40+
},
41+
};
42+
return () => { globalThis.window = prev; };
43+
};
44+
45+
const makeContext = () => {
46+
const calls = [];
47+
let focused = 0;
48+
const aceObj = {
49+
ace_invoked: (val) => calls.push({type: 'invoked', val}),
50+
};
51+
return {
52+
calls,
53+
get focusCount() { return focused; },
54+
ctx: {
55+
ace: {
56+
callWithAce(fn, op, fast) { calls.push({type: 'callWithAce', op, fast}); fn(aceObj); },
57+
focus() { focused++; },
58+
},
59+
},
60+
};
61+
};
62+
63+
describe('toolbarSelect', () => {
64+
describe('config validation', () => {
65+
let restore;
66+
beforeEach(() => { restore = installJqueryStub(makeFakeSelect()); });
67+
afterEach(() => restore());
68+
69+
const ok = () => {
70+
const {ctx} = makeContext();
71+
return {
72+
selector: '#x',
73+
context: ctx,
74+
invoke: (ace, v) => ace.ace_invoked(v),
75+
};
76+
};
77+
78+
it('throws when config is missing', () => {
79+
assert.throws(() => toolbarSelect(), /config object/);
80+
assert.throws(() => toolbarSelect(null), /config object/);
81+
});
82+
83+
it('throws when selector is missing or not a string', () => {
84+
assert.throws(() => toolbarSelect({...ok(), selector: ''}), /selector/);
85+
assert.throws(() => toolbarSelect({...ok(), selector: 42}), /selector/);
86+
});
87+
88+
it('throws when context.ace is absent or missing required methods', () => {
89+
assert.throws(() => toolbarSelect({...ok(), context: {}}), /context\.ace/);
90+
assert.throws(
91+
() => toolbarSelect({...ok(), context: {ace: {callWithAce: () => {}}}}),
92+
/callWithAce \/ focus/);
93+
assert.throws(
94+
() => toolbarSelect({...ok(), context: {ace: {focus: () => {}}}}),
95+
/callWithAce \/ focus/);
96+
});
97+
98+
it('throws when invoke is not a function', () => {
99+
assert.throws(() => toolbarSelect({...ok(), invoke: undefined}), /invoke/);
100+
assert.throws(() => toolbarSelect({...ok(), invoke: 'not-a-fn'}), /invoke/);
101+
});
102+
103+
it('throws when op is provided but empty', () => {
104+
assert.throws(() => toolbarSelect({...ok(), op: ''}), /op must be a non-empty string/);
105+
});
106+
107+
it('throws when coerce is an unknown token', () => {
108+
assert.throws(() => toolbarSelect({...ok(), coerce: 'bigint'}),
109+
/coerce must be a function or one of/);
110+
});
111+
112+
it('throws when onAfterChange is provided but not a function', () => {
113+
assert.throws(() => toolbarSelect({...ok(), onAfterChange: 42}), /onAfterChange/);
114+
});
115+
});
116+
117+
describe('change behaviour', () => {
118+
it("calls invoke with coerced int, resets select, and focuses editor", () => {
119+
const $sel = makeFakeSelect('initial');
120+
const restore = installJqueryStub($sel);
121+
const {ctx, calls} = makeContext();
122+
let captured = ctx; // alias for clarity
123+
124+
toolbarSelect({
125+
selector: '#font-size',
126+
context: ctx,
127+
invoke: (ace, v) => ace.ace_invoked(v),
128+
op: 'insertsize',
129+
});
130+
131+
$sel._fire('24');
132+
restore();
133+
134+
assert.deepStrictEqual(calls, [
135+
{type: 'callWithAce', op: 'insertsize', fast: true},
136+
{type: 'invoked', val: 24},
137+
]);
138+
assert.strictEqual($sel.val(), 'dummy', 'select should be reset to sentinel');
139+
assert.strictEqual(captured === ctx, true);
140+
});
141+
142+
it('still focuses the editor when the coerced value is unusable', () => {
143+
const $sel = makeFakeSelect('initial');
144+
const restore = installJqueryStub($sel);
145+
const ce = makeContext();
146+
147+
toolbarSelect({
148+
selector: '#font-size',
149+
context: ce.ctx,
150+
invoke: (ace, v) => ace.ace_invoked(v),
151+
});
152+
153+
$sel._fire('not-a-number');
154+
restore();
155+
156+
// No edit happened, but focus was restored — the user picked from a
157+
// toolbar control and we don't want keystrokes to land back on it.
158+
assert.strictEqual(ce.calls.length, 0);
159+
assert.strictEqual(ce.focusCount, 1);
160+
assert.strictEqual($sel.val(), 'not-a-number',
161+
'unusable picks must not reset the select — the user can see what they picked');
162+
});
163+
164+
it('supports a function coercer', () => {
165+
const $sel = makeFakeSelect();
166+
const restore = installJqueryStub($sel);
167+
const ce = makeContext();
168+
169+
toolbarSelect({
170+
selector: '#x',
171+
context: ce.ctx,
172+
invoke: (ace, v) => ace.ace_invoked(v),
173+
coerce: (raw) => raw === 'special' ? {marker: true} : null,
174+
});
175+
176+
$sel._fire('special');
177+
restore();
178+
179+
assert.strictEqual(ce.calls.length, 2);
180+
assert.deepStrictEqual(ce.calls[1], {type: 'invoked', val: {marker: true}});
181+
});
182+
183+
it('honours a custom resetValue', () => {
184+
const $sel = makeFakeSelect();
185+
const restore = installJqueryStub($sel);
186+
const ce = makeContext();
187+
188+
toolbarSelect({
189+
selector: '#x',
190+
context: ce.ctx,
191+
invoke: (ace, v) => ace.ace_invoked(v),
192+
resetValue: '__none__',
193+
});
194+
195+
$sel._fire('7');
196+
restore();
197+
198+
assert.strictEqual($sel.val(), '__none__');
199+
});
200+
201+
it('invokes onAfterChange with the coerced value and swallows callback errors', () => {
202+
const $sel = makeFakeSelect();
203+
const restore = installJqueryStub($sel);
204+
const ce = makeContext();
205+
const seen = [];
206+
const consoleErr = console.error;
207+
console.error = () => {}; // silence the expected error log
208+
209+
try {
210+
toolbarSelect({
211+
selector: '#x',
212+
context: ce.ctx,
213+
invoke: (ace, v) => ace.ace_invoked(v),
214+
onAfterChange: (v) => {
215+
seen.push(v);
216+
if (v === 99) throw new Error('boom');
217+
},
218+
});
219+
220+
$sel._fire('5');
221+
$sel._fire('99');
222+
} finally {
223+
console.error = consoleErr;
224+
restore();
225+
}
226+
227+
assert.deepStrictEqual(seen, [5, 99]);
228+
assert.strictEqual(ce.focusCount, 2, 'focus must run for both changes');
229+
});
230+
});
231+
});

0 commit comments

Comments
 (0)