Skip to content

Commit 5ac1e3a

Browse files
fix(library): keyboard-operable tag options and an uncapped vocabulary
Addresses review on the tag picker: - The host vocabulary was normalized with the per-element 50-tag cap, silently dropping every entry past the 50th — so a host with more than 50 test cases could not offer the later ones. Lift the count cap for the vocabulary (`normalizeTags` gains a `maxCount`, `Infinity` for `available`) while keeping trimming, validation, and de-duplication. - The vocabulary options were click-only `<li>`s with no keyboard support. Make each option a native toggle `<button aria-pressed>` in a `role="group"`, so keyboard users can Tab to and Enter/Space a tag. The story now selects a tag by keyboard, and a unit test covers a 50+ vocabulary.
1 parent d19bd7e commit 5ac1e3a

6 files changed

Lines changed: 70 additions & 23 deletions

File tree

library/lib/components/popovers/TagPicker.tsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -112,19 +112,19 @@ export const TagPicker: React.FC<TagControlProps> = ({
112112
style={portalThemeVars}
113113
>
114114
{options.length > 0 && (
115-
<ul
115+
<div
116116
data-slot="tag-picker-list"
117117
className="apollon-tag-picker__list"
118-
role="listbox"
118+
role="group"
119119
aria-label={t.editTagsFor(subject)}
120120
>
121121
{options.map((tag) => {
122122
const selected = tags.includes(tag)
123123
return (
124-
<li
124+
<button
125125
key={tag}
126-
role="option"
127-
aria-selected={selected}
126+
type="button"
127+
aria-pressed={selected}
128128
data-slot="tag-picker-option"
129129
className="apollon-tag-picker__option"
130130
onClick={() => toggle(tag)}
@@ -137,10 +137,10 @@ export const TagPicker: React.FC<TagControlProps> = ({
137137
aria-hidden="true"
138138
/>
139139
<span>{tag}</span>
140-
</li>
140+
</button>
141141
)
142142
})}
143-
</ul>
143+
</div>
144144
)}
145145

146146
{options.length === 0 && !allowCreate && (

library/lib/styles/app.css

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,8 +1373,15 @@ svg.react-flow__connectionline {
13731373
display: flex;
13741374
align-items: center;
13751375
gap: 6px;
1376+
width: 100%;
1377+
margin: 0;
13761378
padding: 6px 8px;
1379+
font: inherit;
13771380
font-size: 0.8125rem;
1381+
text-align: left;
1382+
color: inherit;
1383+
background: transparent;
1384+
border: 0;
13781385
border-radius: var(--apollon-radius-sm, 4px);
13791386
cursor: pointer;
13801387
}
@@ -1383,6 +1390,11 @@ svg.react-flow__connectionline {
13831390
background-color: var(--apollon-hover-neutral, rgba(0, 0, 0, 0.05));
13841391
}
13851392

1393+
.apollon-tag-picker__option:focus-visible {
1394+
outline: 2px solid var(--apollon-primary, #3e8acc);
1395+
outline-offset: -2px;
1396+
}
1397+
13861398
.apollon-tag-picker__option > span {
13871399
overflow: hidden;
13881400
text-overflow: ellipsis;

library/lib/utils/tagUtils.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ type TaggableData = { tags?: unknown; [key: string]: unknown }
1919
* preserved). Idempotent. Takes `unknown` because models arrive from hosts as
2020
* untrusted JSON, so a `tags` entry may be any shape.
2121
*/
22-
export function normalizeTags(raw: unknown): string[] {
22+
export function normalizeTags(
23+
raw: unknown,
24+
maxCount = MAX_TAGS_PER_ELEMENT
25+
): string[] {
2326
if (!Array.isArray(raw)) return []
2427
const seen = new Set<string>()
2528
const result: string[] = []
@@ -32,7 +35,7 @@ export function normalizeTags(raw: unknown): string[] {
3235
if (seen.has(tag)) continue
3336
seen.add(tag)
3437
result.push(tag)
35-
if (result.length >= MAX_TAGS_PER_ELEMENT) break
38+
if (result.length >= maxCount) break
3639
}
3740
return result
3841
}
@@ -105,7 +108,9 @@ export const DISABLED_TAG_CONFIG: TagConfig = {
105108
export function resolveTagConfig(input?: boolean | TagOptions): TagConfig {
106109
if (!input) return DISABLED_TAG_CONFIG
107110
if (input === true) return { enabled: true, available: [], allowCreate: true }
108-
const available = normalizeTags(input.available ?? [])
111+
// The 50-tag cap bounds one element's tags, not the offered vocabulary — a
112+
// host may legitimately expose one tag per test case, well past 50.
113+
const available = normalizeTags(input.available ?? [], Infinity)
109114
return {
110115
enabled: true,
111116
available,

library/tests/unit/TagPicker.test.tsx

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,20 +71,23 @@ describe("TagPicker free-form (tags: true)", () => {
7171
describe("TagPicker with a host vocabulary", () => {
7272
const available = ["testAttributes[Animal]", "testMethods[Animal]"]
7373

74-
it("lists the whole vocabulary as a checklist, no search box", () => {
74+
it("lists the whole vocabulary as focusable buttons, no search box", () => {
7575
renderPicker([], { available })
7676
openPopover()
77+
// Options are native buttons (keyboard-operable), not click-only list items.
7778
expect(
78-
within(screen.getByRole("listbox")).getAllByRole("option")
79+
within(screen.getByRole("group")).getAllByRole("button")
7980
).toHaveLength(2)
80-
// Pick-only: no add field, no filter.
81-
expect(screen.queryByLabelText("Add tag")).not.toBeInTheDocument()
81+
// Pick-only: no add field.
82+
expect(screen.queryByLabelText("New tag")).not.toBeInTheDocument()
8283
})
8384

8485
it("toggles a vocabulary choice on", () => {
8586
const onChange = renderPicker([], { available })
8687
openPopover()
87-
fireEvent.click(screen.getByText("testAttributes[Animal]"))
88+
fireEvent.click(
89+
screen.getByRole("button", { name: "testAttributes[Animal]" })
90+
)
8891
expect(onChange).toHaveBeenCalledWith(["testAttributes[Animal]"])
8992
})
9093

@@ -104,10 +107,11 @@ describe("TagPicker with a host vocabulary", () => {
104107
expect(
105108
screen.getByRole("button", { name: "Remove tag legacyTag" })
106109
).toBeInTheDocument()
107-
// …and the option shows inside it, checked.
110+
// …and the option shows inside it, pressed.
108111
openPopover()
109-
expect(
110-
within(screen.getByRole("listbox")).getByText("legacyTag")
111-
).toBeInTheDocument()
112+
const option = within(screen.getByRole("group")).getByRole("button", {
113+
name: "legacyTag",
114+
})
115+
expect(option).toHaveAttribute("aria-pressed", "true")
112116
})
113117
})

library/tests/unit/tagUtils.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,18 @@ describe("resolveTagConfig", () => {
185185
})
186186
})
187187

188+
it("does not cap the vocabulary at the per-element tag limit", () => {
189+
// The 50-tag cap bounds one element's tags, not the offered vocabulary — a
190+
// host may expose one tag per test case, well past 50.
191+
const many = Array.from(
192+
{ length: MAX_TAGS_PER_ELEMENT + 25 },
193+
(_, i) => `test${i}`
194+
)
195+
expect(resolveTagConfig({ available: many }).available).toHaveLength(
196+
MAX_TAGS_PER_ELEMENT + 25
197+
)
198+
})
199+
188200
it("respects an explicit allowCreate over the default", () => {
189201
expect(resolveTagConfig({ available: ["a"], allowCreate: true })).toEqual({
190202
enabled: true,

standalone/webapp/src/stories/editor/ElementTags.stories.tsx

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,27 @@ export const Authoring: Story = {
8787
canvas.getByRole("button", { name: "Remove tag testAttributes[Animal]" })
8888
).toBeVisible()
8989

90-
// Open the attribute's tag picker and add a tag off the vocabulary.
91-
// The popover portals to <body>, so query it through `screen`, not `canvas`.
90+
// Open the attribute's tag picker. The popover portals to <body>, so query
91+
// it through `screen`, not `canvas`.
9292
await userEvent.click(
9393
canvas.getByRole("button", { name: "Tags for attribute" })
9494
)
95-
const addField = await screen.findByLabelText("New tag")
96-
await userEvent.type(addField, "testName{Enter}")
95+
96+
// Vocabulary options are keyboard-operable: focus one and toggle with Enter.
97+
const option = await screen.findByRole("button", {
98+
name: "testMethods[Animal]",
99+
})
100+
option.focus()
101+
await userEvent.keyboard("{Enter}")
102+
await expect(
103+
canvas.getByRole("button", { name: "Remove tag testMethods[Animal]" })
104+
).toBeVisible()
105+
106+
// And a new tag can be added through the field below the list.
107+
await userEvent.type(
108+
await screen.findByLabelText("New tag"),
109+
"testName{Enter}"
110+
)
97111
await expect(
98112
canvas.getByRole("button", { name: "Remove tag testName" })
99113
).toBeVisible()

0 commit comments

Comments
 (0)