fix: [OCISDEV-115] address post-merge review on coloured tags - #12923
Merged
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
gauravsoni119
force-pushed
the
fix/ocisdev-115-coloured-tags-review
branch
from
September 10, 2026 09:55
ca4e3f4 to
588af91
Compare
9 tasks
mzner
previously approved these changes
Sep 10, 2026
gauravsoni119
force-pushed
the
fix/ocisdev-115-coloured-tags-review
branch
from
September 10, 2026 11:18
588af91 to
07e1e2d
Compare
LukasHirt
requested changes
Sep 10, 2026
Five review comments landed on #12892 after it merged. **The tag overflow popover dismissed itself on click.** `mode="hover"` maps to tippy's `mouseenter focus` trigger, and `hideOnClick` is hardcoded true in `OcDrop`. tippy only skips its document-press hide when the trigger contains `click`, so a click on the button fell through to `hide()` and set `didHideDueToDocumentMouseDown`, which then blocked the focus trigger from reopening it — the pointer had to leave and come back. `mode="click"` fixes it without touching the design system: tippy toggles a click-triggered instance, so the button opens and closes it, Enter/Space work because a native button activation dispatches a click, and Escape still closes via the `hideOnEsc` plugin with a click able to reopen it. Under `hover` that last one was a dead end — Escape hid the popover and no new focus event could follow. The trade is that hovering no longer reveals the tags; a disclosure that answers to the keyboard is worth more than a hover shortcut. That click also had to stop propagating: the button sits inside the `<tr>` that `OcTable` highlights on click, so opening the popover selected the file as well. tippy's listener is on the same element, so stopping propagation still toggles the popover. `is-nested` goes with it. It was set purely to suppress the `hideAll` that ran when a pointer crossed the button, and pointer movement no longer opens anything. Dropping it also un-silences `showDrop`/`hideDrop`, which `OcDrop` only emits for drops that are not nested. **The button claimed to be a popover trigger without saying so.** It now carries `aria-haspopup` and an `aria-expanded` driven by those two events. tippy manages `aria-expanded` itself only when the reference does not already have one (`hasAriaExpanded` is read once at create time), and Vue renders the attribute before `OcDrop` mounts, so there is one owner, not two. **The accessible name dropped the visible text.** WCAG 2.5.3 Label in Name wants the visible `+ 3` inside the name; `Show 3 more tags` left speech input with nothing to match. The count now leads the string. **The read-only tag lock was invisible to screen readers.** No `accessible-label` means `OcIcon` renders it `aria-hidden`, so the only signal that a tag cannot be removed was the absent deselect button. **Themes inherited no tag palette.** `tagColorsList` was missing from `defaults.designTokens`, so a deployment serving its own theme.json gets grey chips. Seeding the defaults with the light palette turned out to be the wrong place for it: every other value in that block is an empty-string placeholder, because supplying colours is a theme's responsibility. The defaults stay colourless. `useTagColor` already returns -1 without a list and `OcTag` then renders the plain chip, which is what tags looked like before this feature.
gauravsoni119
force-pushed
the
fix/ocisdev-115-coloured-tags-review
branch
from
September 10, 2026 12:14
07e1e2d to
31033ed
Compare
LukasHirt
approved these changes
Sep 10, 2026
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
4 participants
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.
Address post-merge review feedback on the coloured tags feature.