fix(transcription): harden re-edit pencil hit-testing + disable sync (#628 review)
Addresses the clean-agent review of PR #717: - C1: the hidden pencil was opacity-0 only, which still hit-tests; its 44px box overhangs adjacent text, so a click in the gap between two mentions could land on the invisible button and spuriously open the dropdown (AC-8 hole). Add pointer-events-none while hidden, re-enabled with the opacity reveal on hover/focus. - C2/N1: editor.setEditable() emits "update", not a ProseMirror transaction, so the NodeView's 'transaction' listener missed a mid-session disable flip (stale aria-disabled/tabindex; the comment was wrong). Listen on 'update' instead — which also skips selection-only changes, so it fires far less often. - N2: track the node across update() so the pencil opens with the live displayName (hardening; relink only swaps personId today). Tests: structural guard that the hidden pencil is pointer-events-none + reveals, and a mid-session disable-flip test (fixture gains an onReady setDisabled hook). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1333,3 +1333,54 @@ describe('PersonMentionEditor — #628 editing announce', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #628 review fixes: hidden-pencil hit-testing + mid-session disable ───────
|
||||
|
||||
describe('PersonMentionEditor — #628 review fixes', () => {
|
||||
it('keeps the hidden pencil out of hit-testing (pointer-events-none until revealed)', async () => {
|
||||
// opacity-0 alone still hit-tests, and the 44px button overhangs adjacent
|
||||
// text — so a click in the gap between mentions would otherwise land on the
|
||||
// invisible pencil and open the dropdown (AC-8). It must be pointer-events-none
|
||||
// while hidden and only become clickable together with the opacity reveal.
|
||||
renderHost({ value: '@Aug ', mentionedPersons: [{ personId: 'p-aug', displayName: 'Aug' }] });
|
||||
await vi.waitFor(() =>
|
||||
expect(document.querySelector('[data-type="mention"] button')).not.toBeNull()
|
||||
);
|
||||
const btn = document.querySelector('[data-type="mention"] button') as HTMLElement;
|
||||
expect(btn.className).toContain('pointer-events-none');
|
||||
expect(btn.className).toContain('group-hover/mention:pointer-events-auto');
|
||||
expect(btn.className).toContain('group-focus-within/mention:pointer-events-auto');
|
||||
});
|
||||
|
||||
it('re-syncs the pencil to inert when the editor is disabled mid-session', async () => {
|
||||
// editor.setEditable() emits "update", not a transaction — the NodeView must
|
||||
// listen on "update" or a runtime disable flip leaves the pencil stale.
|
||||
let setDisabled!: (value: boolean) => void;
|
||||
render(PersonMentionEditorHost, {
|
||||
initialValue: '@Aug ',
|
||||
initialMentions: [{ personId: 'p-aug', displayName: 'Aug' }],
|
||||
disabled: false,
|
||||
onChange: () => {},
|
||||
onReady: (api: { setDisabled: (value: boolean) => void }) => {
|
||||
setDisabled = api.setDisabled;
|
||||
}
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
const btn = document.querySelector(
|
||||
'[data-type="mention"] button'
|
||||
) as HTMLButtonElement | null;
|
||||
expect(btn).not.toBeNull();
|
||||
expect(btn!.disabled).toBe(false);
|
||||
});
|
||||
|
||||
setDisabled(true);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
const btn = document.querySelector('[data-type="mention"] button') as HTMLButtonElement;
|
||||
expect(btn.disabled).toBe(true);
|
||||
expect(btn.getAttribute('aria-disabled')).toBe('true');
|
||||
expect(btn.tabIndex).toBe(-1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user