Repository navigation
Notes: stop the highlight tint from deepening, emphasize markers with an underline - #80544
adamsilverstein wants to merge 2 commits into
Conversation
…derline The per-author tint sits behind the marker's text, so it is subtracted from whatever text/background contrast the theme provides, and the canvas background comes from theme.json, so the composited result cannot be measured from CSS. Doubling the alpha on hover, focus, and selection doubled that loss: on a mid-tone canvas an orange-authored marker dropped from a passing 4.54:1 to 2.67:1. Hold the tint at the rest alpha in every state and carry emphasis with an opaque underline in the author's color instead, which reads more strongly than a heavier wash without touching what sits behind the text.
adamsilverstein
added
Collaborative Workflows
|
Size Change: +58 B (0%) Total Size: 7.74 MB 📦 View Changed
|
|
cc: @fcoveram for review as well |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in 648981b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/29870507365
|
|
Tested on WordPress Playground, confirms the fix works as expected. The tint stays the same in both rest and selected states, and selecting/hovering the marker adds an underline in the author's color instead of deepening the background. One side note: when selecting a note, the rest of the text gets a dimming overlay. Not sure if this is expected in this environment or pre-existing behavior unrelated to this PR, flagging in case it's useful. Screen.Recording.2026-07-22.at.7.42.15.AM.mov |
|
Thanks, @adamsilverstein! Let's get some design feedback before committing to this. |
|
If the contrast is insufficient in the present state, then as suggested, an underline is a good next step. I think we can refine the thickness, we want to not confuse with hyperlinks, I wonder if 1.5px is right. One thing though: can we make the underline the same color as the user avatar and marking itself? Only full-opacity. I guess that due to the possible light and dark modes, using currentColor is the most safe. But can we then at least try to seed in some of the highlight color through color-mix? Worth testing more in-depth, but something like |
Sure I can try that. I'm going to bring this into the combined PR suggested by @Mamaduka so we can see all these related changes working together. |
|
Closing in favor of the combined PR: |
4 participants
What?
Holds an inline note marker's tint at one low alpha in every state, and moves the hover / focus / selected emphasis to an opaque underline in the author's color.
Fixes #80543, raised by @joedolson in #80498 (comment).
Why?
The tint is painted behind the marker's text, so it is subtracted from whatever text/background contrast the theme already provides, and there is no contrast assessment anywhere in that path. Worse, the alpha doubled from
0x40(25%) to0x80(50%) on hover, focus, and selection, so the strong state cost roughly twice as much as the resting one.WCAG 2.1 contrast of the theme's own text color against the tinted background, worst palette member per state:
#fff/#1e1e1e)#1e1e1e/#fff)#767676/#fff)#f5e6c8/#4a3b1f)The default editor is comfortable throughout. The damage lands on themes whose own pair is already near the 4.5:1 floor, where selecting a note could drop passing text to 2.67:1.
Why not
color: contrast-color(...)That was the original suggestion, and it backfires here.
contrast-color()only sees the color it is handed; it cannot know the backdrop, and through a 25% tint the backdrop is most of what the reader sees. Deriving the text color from the raw author color picks wrong in exactly the common case:#6F42C1#0F766E#FBBF24contrast-color()is sound over an opaque background, which a translucent highlight is not. (Browser support is not the obstacle: it went Baseline Newly Available in April 2026 and degrades to inherited color.)So instead of trying to compute a text color we cannot compute, this bounds what the highlight is allowed to cost: one fixed, small tint, and emphasis that does not sit behind the glyphs.
How?
In
note-highlight-styles.js:REST_ALPHA/ACTIVE_ALPHAcollapse to a singleTINT_ALPHA(0x40, unchanged from today's resting value).:hover,:focus-within, and the selected-note rule now emittext-decoration-line: underlinewithtext-decoration-colorset to the opaque author color,0.125emthick at a0.15emoffset.text-decorationfollows text across line wraps natively, whichbox-shadoworborder-bottomon an inline element would not.An opaque line is a stronger signal than a heavier wash, so the emphasis state reads more clearly while costing the text nothing.
Scope: inline markers only, on trunk. #80498 applies the same helper to whole text blocks and inherits
TINT_ALPHAon merge; the right block-level equivalent of the underline is an open question there and in #80543, since a paragraph-wide underline is not appropriate.Testing Instructions
Automated
npm run test:unit packages/editor/src/components/collab-sidebar/test/note-highlight-styles.js- 10 passing, including a new case asserting no rule can emit a background alpha above the rest value.npm run test:e2e -- test/e2e/specs/editor/various/block-notes.spec.js- 45 passing on a reset environment. The selected-marker case was rewritten to assert the underline and to assert the tint stays put; run against trunk's implementation first it fails, as expected.Screenshots
Selected inline marker, before (50% wash)
after (25% wash plus underline):
Open question for review
@jasmussen @joedolson - the underline is deliberately conservative. If the emphasis reads too quietly for connecting a canvas marker to its sidebar thread, the thickness and offset are the dials, and #80543 also asks whether the at-rest marking should move off the background entirely.