Allow selecting unlabeled HighlightedText tokens - #13815
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://huggingface.co/buckets/gradio/pypi-previews/resolve/9b4659e0013eae561f66177a4d8aa05383202276/gradio-6.26.0-py3-none-any.whlInstall Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@9b4659e0013eae561f66177a4d8aa05383202276#subdirectory=client/python"Import Gradio JS Client from this PR via CDN import { Client } from "https://huggingface.co/buckets/gradio/npm-previews/resolve/9b4659e0013eae561f66177a4d8aa05383202276/browser.js"; |
🦄 change detectedThis Pull Request includes changes to the following packages.
|
Before / after Spaces
Both Spaces run byte-identical
The category tab also includes mixed labeled/unlabeled text, a legend, custom colors, and a transparent label for visual regression checks. |
There was a problem hiding this comment.
🟡 Changes recommended
The manually authored changeset conflicts with the repository’s automated changeset policy.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Enables unlabeled HighlightedText tokens to dispatch select events while preserving static output behavior.
Changes:
- Propagates
_selectableto token rendering. - Adds mouse, Enter, and Space activation with button semantics.
- Adds regression and shared-prop tests.
File summaries
| File | Description |
|---|---|
js/highlightedtext/types.ts |
Adds the selectable prop type. |
js/highlightedtext/Index.svelte |
Forwards selectable state. |
js/highlightedtext/shared/HighlightedText.svelte |
Implements selectable token behavior. |
js/highlightedtext/highlightedtext.test.ts |
Tests selection and keyboard activation. |
.changeset/every-laws-send.md |
Adds manual release metadata. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| --- | ||
| "@gradio/highlightedtext": patch | ||
| "gradio": patch | ||
| --- | ||
|
|
||
| fix:Allow selecting unlabeled HighlightedText tokens |
There was a problem hiding this comment.
This changeset was generated by Gradio’s trigger-changeset GitHub Action in commit 4708dce, rather than authored manually. Keeping it is therefore consistent with the repository policy.
hysts
left a comment
There was a problem hiding this comment.
Label entry is broken in interactive=True on this branch. Details inline. Numbers are this PR's wheel against released gradio==6.26.0 in Chromium, and git log gradio@6.26.0..main -- js/highlightedtext/ is empty, so the release is the base:
| 6.26.0 | this PR | |
|---|---|---|
type NAMED ENTITY into a label editor |
NAMED ENTITY |
NAMEDENTITY |
select events during that one edit |
1 | 3 |
20 keydowns on a token, 19 with repeat: true |
handler runs 0 times | 20 times |
| 47-word unlabeled text, tokens in the tab order | 0 | 47 |
The scores branch needs the same guard as the first inline comment. Its span keydown (:320-331) wraps its own LabelInput (:336), so Enter there reopens the editor. Measured with [("alpha", 0.5), ("beta", -0.5)] and interactive=True, still open after Enter, on this PR and on 6.26.0. Fixing one span and not the other leaves half the component on the old behaviour.
Repro on this PR's wheel (4708dce3a), one tab per point below: https://huggingface.co/spaces/hysts-debug/pr-13815-highlightedtext
| token_is_selectable && | ||
| (e.key === "Enter" || e.key === " ") | ||
| ) { | ||
| e.preventDefault(); |
There was a problem hiding this comment.
LabelInput renders inside this span (:255-266) and does not stop propagation, so Enter and Space typed into the editor reach here too. token_is_selectable is always true for the token being edited, so preventDefault() eats the character and handle_token_select fires on top of it. Typing NAMED ENTITY gives NAMEDENTITY and 3 select events instead of 1. It happens with no .select() listener attached, so it reaches every interactive HighlightedText.
Separately, activation is on keydown with no repeat check, so the handler runs once per auto-repeat. Twenty keydowns, nineteen repeat: true, run it twenty times, counted through defaultPrevented. The queue coalesces events fired that fast so fewer reach the app, but a role="button" should activate once per press either way, and every repeat is also a swallowed keystroke.
Both are one line:
onkeydown={(e) => {
if (e.target !== e.currentTarget || e.repeat) return;
...
}}Svelte's handle_event_propagation reassigns currentTarget per hop, so the guard holds under delegation.
The same guard also ends the Enter-reopen already on main, where LabelInput sets label_to_edit = -1 and this handler sets it straight back. It comes for free with this line.
| (e.key === "Enter" || e.key === " ") | ||
| ) { | ||
| e.preventDefault(); | ||
| handle_token_select(i, token, class_or_confidence); |
There was a problem hiding this comment.
token is the whole entry, but {#each lines as line, j} renders one focusable span per line. A single entry ("first line\nsecond line", None) becomes two buttons, both reporting index=0, value=['first line\nsecond line', None]. On 6.26.0 it is zero buttons, so this is newly reachable, and unlabeled runs are where multi-line text lives.
Passing line instead of token, or not making each line its own button, settles it.
| role={class_or_confidence !== null ? "button" : undefined} | ||
| tabindex={class_or_confidence !== null ? 0 : undefined} | ||
| role={token_is_selectable ? "button" : undefined} | ||
| tabindex={token_is_selectable ? 0 : undefined} |
There was a problem hiding this comment.
Every token joins the page tab order. The 47-word sample from #11991 goes from 0 tab stops to 47, and crossing the component with Tab from 1 press to 48. Tab 4 of the Space is that sample if you want to feel it.
The same question from the other side, already true on main: a static output with no listener still gives every labeled token a role="button" that does nothing.
Roving tabindex answers both: the container takes one tab stop and arrow keys move between tokens. selectable || (interactive && class_or_confidence !== null) closes the second half on its own. Both change focus behaviour in apps that already ship, so whether that belongs here or somewhere else is your call.
| }); | ||
|
|
||
| describe("Select events", () => { | ||
| test("dispatches select when clicking an unlabeled selectable token", async () => { |
There was a problem hiding this comment.
Nothing covers interactive=True, so CI stays green while label entry is broken. Un-skipping the two existing labeled-token tests would not catch it either: one sets the value with fireEvent.input and never sends a keydown, the other only clicks.
test("typing a label with a space keeps the space and fires no extra select", async () => {
const { getByText, container, listen } = await render(HighlightedText, {
interactive: true,
loading_status,
_selectable: true,
value: [{ token: "editable", class_or_confidence: "original" }]
});
const select = listen("select");
await fireEvent.click(getByText("editable"));
const from_the_click = select.mock.calls.length;
const input = container.querySelector(".label-input") as HTMLInputElement;
input.focus();
await event.keyboard("past tense");
// the input opens with the existing label text before the caret
expect(input.value).toContain("past tense");
expect(select.mock.calls.length).toBe(from_the_click);
});Fails twice on this branch.
…ghlightedtext-allow-0-labels-or-transparent
|
Thanks @hysts for the fix. I fixed the label entry bug you caught, thanks, and confirmed manually. I'm not sure I completely understood the roving/tab comment but I've improved keyboard usage as well. I updated the before/after demo as well to make it easy to confirm the fixes: https://huggingface.co/spaces/abidlabs/gradio-11991-after |
hysts
left a comment
There was a problem hiding this comment.
Thanks for the update @abidlabs! Everything from last round is fixed and the new tests pin each case. The keyboard handling is the roving tabindex I was describing, so that part is settled, and carrying the guard over to the scores span was the piece I would otherwise have had to ask for.
LGTM. Three small notes inline. None of them block, so take or leave them as you see fit.
| {@const lines = token.split("\n")} | ||
| {@const token_is_selectable = is_token_selectable(class_or_confidence)} | ||
| {#each lines as line, j} | ||
| {#if show_whitespaces ? line !== "" : line.trim()} |
There was a problem hiding this comment.
is_visible_line (:199-200) is the same predicate, and it is what category_selectable_token_keys uses to build the roving key list. The two agree today. If they ever drift, selectable_keys[0] can name a line that renders nothing, and then get_roving_tabindex hands every rendered token -1 and the component quietly drops out of the tab order, with no error anywhere. Calling is_visible_line(line) here keeps the key list and the DOM derived from one source.
| } | ||
| } | ||
|
|
||
| function handle_score_token_select( |
There was a problem hiding this comment.
Worth a line saying why this is not handle_token_select. The categories one always dispatches select and then opens the editor, this one does one or the other, so an interactive scores token never emits select. That is main's behaviour (the old scores click had the same either/or), but with the two functions now side by side the difference reads as an oversight.
| ).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test.skip("dispatches select event when clicking highlighted token", async () => { |
There was a problem hiding this comment.
These two are the only coverage for select on a labeled token, and that path moved in this PR: clicks now go through handle_token_select and pass line rather than token. The only reason they are skipped is the pre-listen payload shape, so the fix is the same idiom the new tests use:
expect(select).toHaveBeenCalledWith({ index: 0, value: ["clickable", "label"] });Both tokens are single-line, so line === token and the expectations hold as written.
…ghlightedtext-allow-0-labels-or-transparent
Description
Allow
gr.HighlightedTexttokens with aNonelabel to dispatch.select()events when a select listener is registered. The frontend now propagates_selectable, gives selectable unlabeled tokens accessible button semantics, and supports mouse, Enter, and Space activation without making plain static output selectable.Closes: #11991
AI Disclosure
Testing and Formatting Your Code
CI=1 pnpm exec vitest run --config .config/vitest.config.ts js/highlightedtext/highlightedtext.test.tspnpm format:checkpnpm exec eslint -c .config/eslint.config.js js/highlightedtext/Index.svelte js/highlightedtext/shared/HighlightedText.svelte js/highlightedtext/types.tsMinimal reproduction (the local file was not committed):