fix(desktop): stop mention chips from swallowing the caret - #29
Merged
Conversation
Clicking just after a mention in the composer put the caret *inside* the handle, so the next keystroke turned "@bob " into "@bobx". The cause is that a mention chip is an inline decoration over live, editable text, so its painted box doubles as a caret hit target. `--inline-chip-padding-inline` (0.25rem) paints 4px of chip past the last glyph; a click in that band is visually after the name but resolves to the position before the trailing space. Measured in the composer: the trap band is exactly the inline padding -- 4.0px before, 0.0px after. Display mode is not involved; `inline` and `inline-flex` hit-test identically, so this is not fixed by unsetting the flex layout the composer inherits from `.message-markdown`. Zeroing the variable rather than the padding keeps the agent chip's derived geometry consistent: `padding-left` stays icon + gap and the `::before` robot icon moves flush to the chip's left edge. Rendered messages are untouched -- they are not editable, so the band costs nothing there and the padded pill is the intended look. The E2E asserts the structural invariant (the chip's box may not extend past its last glyph) as well as the behaviour, because the behavioural half alone is a pixel away from the genuinely ambiguous end-of-word click that every editor resolves into the word.
… space's width The probe clicked two pixels past the last glyph and expected the caret to land after the trailing space. That holds only while half a space is under two pixels wide, which is a macOS font metric — on the Linux runners the same click resolved before the space and the test failed three times in a row on a fix that works. The regression is the chip's painted box extending past its last glyph, and that measurement is font-independent, so it stays as the pin. The click now probes three quarters into the space glyph, measured, where every platform agrees the caret belongs after it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Clicking just after a mention in the composer put the caret inside the handle, so the next keystroke turned
@bobinto@bobx.A mention chip is an inline ProseMirror decoration over live, editable text, so its painted box doubles as a caret hit target.
--inline-chip-padding-inline(0.25rem,markdown.css) paints 4px of chip past the last glyph. A click in that band is visually after the name but resolves to the document position before the trailing space.Why not the obvious fix
The composer's contenteditable carries
MESSAGE_MARKDOWN_CLASS, so it inherits.message-markdown .mention-chip { display: inline-flex; ... }. The tempting fix is to unset that. Measured against a live composer chip, it changes nothing:inline-flex,padding: 3px 4px 2px)display: inline; padding: 0 2pxdisplay: inline; padding: 0 4pxpadding-inline: 0(either display)The trap band is exactly the inline padding.
inlineandinline-flexhit-test identically.The change
One declaration, scoped to the editable surfaces:
Zeroing the variable rather than the padding keeps the agent chip's derived geometry consistent — its
padding-leftstaysicon + gapand the::beforerobot icon simply moves flush to the chip's left edge.Rendered messages are untouched: they are not editable, so the band costs nothing there and the padded pill is the intended look.
Test plan
New E2E in
desktop/tests/e2e/mentions.spec.ts, verified red onmain(band= 4, expected ≤ 0.5) and green here. It asserts both halves:The behavioural half alone is not sufficient: it sits one pixel from the genuinely ambiguous end-of-word click that every editor resolves into the word, so the invariant is what pins the regression.
pnpm lintclean,tsc --noEmitcleanmentions.spec.ts+team-mentions.spec.ts+messaging.spec.ts+channel-composer-overflow.spec.ts: 99 passed. The one failure (messaging.spec.ts→ "shows your avatar when profile avatar is set") reproduces unchanged onmainand is unrelated.