Make EmergencyBadge tooltip work on touch devices and simplify#442
Merged
Make EmergencyBadge tooltip work on touch devices and simplify#442
Conversation
…mobile tap support
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughEmergencyBadge.vue narrows its Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/emergencyaccess/EmergencyBadge.vue`:
- Around line 9-12: The popover dismisses when the cursor crosses the 0.5rem gap
because only the trigger handles mouseenter/mouseleave; update
EmergencyBadge.vue to also track hover on the popover panel by attaching the
same handlers (showIfHover, hideIfHover) to the panel element and implement a
short close delay in hideIfHover using a timeout (store e.g.
this.hoverCloseTimer) that is cleared in showIfHover so brief exits (traversing
the gap) don't immediately close the panel; alternatively remove the margin gap
if adding panel listeners is undesirable.
- Around line 3-16: The popover panel in EmergencyBadge.vue is not associated
with the trigger button, so screen readers won't read the popover message; add a
stable unique id for the popover (e.g., computed/constant like popoverId) and
ensure the popover DOM node renders with that id, then when showing the popover
(in methods that call showIfHover/showPopover/toggleOnTouch) set the button's
aria-describedby to that id and remove it when hiding (hideIfHover/hide popover)
so assistive tech can link the trigger to the panel; update any showPopover/hide
logic to manage this aria attribute lifecycle and ensure the template binds
:aria-describedby="isOpen ? popoverId : null" (or equivalent) so the association
is always correct.
- Line 13: The button with `@click.stop`="toggleOnTouch" is not recognized as the
popover invoker on touch devices, causing a light-dismiss race; fix by adding a
popovertarget attribute on that button pointing to the popover's id (ensure the
popover element has the same id) so the browser recognizes the invoker
declaratively, OR modify the event flow by capturing pointerdown on the button
to track the current open state and have toggleOnTouch consult that flag (update
toggleOnTouch and add a pointerdown handler) to avoid reopening after
light-dismiss.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a27d8a9-9060-4fb4-bde7-e55ac8548fca
📒 Files selected for processing (1)
frontend/src/components/emergencyaccess/EmergencyBadge.vue
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue`:
- Around line 100-105: The "broken" badge condition is inverted: change the
EmergencyBadge usage so it renders when isBroken(vault) (not !isBroken(vault))
and set its type to "error" to match VaultList.vue; keep the same i18n keys
(emergencyAccess.badge.broken.title and …broken.message) and update the
EmergencyBadge props (type, v-if) accordingly so the badge shows only for vaults
where isBroken(vault) is true (i.e., vault.requiredEmergencyKeyShares >
emergencyAccessMembers(vault).length).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de8c3a35-469c-4326-8d82-74f4ea6fa969
📒 Files selected for processing (3)
frontend/src/components/VaultList.vuefrontend/src/components/emergencyaccess/EmergencyAccessVaultList.vuefrontend/src/components/emergencyaccess/EmergencyBadge.vue
mindmonk
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/components/emergencyaccess/EmergencyBadge.vue (1)
13-21:⚠️ Potential issue | 🟠 MajorTooltip not announced to assistive technology — add
aria-describedbylinking trigger to panel.The panel correctly carries
role="tooltip", but nothing on the trigger references it. The trigger only exposesaria-label="title", so screen readers announce the title but never themessage. Generate an id for the panel and reference it from the trigger'saria-describedby.♿ Suggested fix
<script setup lang="ts"> import { computed, useId } from 'vue'; import { ExclamationTriangleIcon } from '@heroicons/vue/24/solid'; const props = defineProps<{ type: 'warning' | 'error'; title: string; message: string; }>(); -const anchor = `--ea-anchor-${useId()}`; +const id = useId(); +const anchor = `--ea-anchor-${id}`; +const panelId = `ea-tooltip-${id}`; const isError = computed(() => props.type === 'error'); </script><div + :id="panelId" class="tooltip-panel px-2 py-1 rounded shadow-sm border text-xs hyphens-auto"(plus
:aria-describedby="panelId"on the trigger as shown in the previous comment)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/emergencyaccess/EmergencyBadge.vue` around lines 13 - 21, Add a unique id for the tooltip panel and wire it to the trigger via aria-describedby: create a component-level panelId (e.g. in data() / setup() or a computed using the component _uid) and bind it as :id="panelId" on the div that has role="tooltip" (the element showing title/message and using props title, message, isError, anchor), then update the trigger element (the element that currently only has aria-label="title") to include :aria-describedby="panelId" so assistive tech will read the tooltip message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/emergencyaccess/EmergencyBadge.vue`:
- Around line 3-11: Replace the non-semantic span used as the interactive
trigger in EmergencyBadge.vue with a proper <button type="button">: remove the
tabindex attribute, keep the same class list but drop or change cursor-default
to a pointer (or omit it) so iOS Safari can focus on tap, preserve :class
binding for isError, keep :style="{ anchorName: anchor }" and
:aria-label="title", and continue rendering the ExclamationTriangleIcon inside
the button so existing styling and focus-visible/ring behavior remain intact;
ensure the component still exports/uses isError, anchor, and title props as
before.
---
Duplicate comments:
In `@frontend/src/components/emergencyaccess/EmergencyBadge.vue`:
- Around line 13-21: Add a unique id for the tooltip panel and wire it to the
trigger via aria-describedby: create a component-level panelId (e.g. in data() /
setup() or a computed using the component _uid) and bind it as :id="panelId" on
the div that has role="tooltip" (the element showing title/message and using
props title, message, isError, anchor), then update the trigger element (the
element that currently only has aria-label="title") to include
:aria-describedby="panelId" so assistive tech will read the tooltip message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9b181b5-7361-48bb-9aa7-e1a00d18574b
📒 Files selected for processing (2)
frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vuefrontend/src/components/emergencyaccess/EmergencyBadge.vue
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
frontend/src/components/emergencyaccess/EmergencyBadge.vue (1)
3-20:⚠️ Potential issue | 🟠 MajorAssociate the tooltip panel with the trigger for screen readers.
role="tooltip"on Line 17 is currently disconnected from the button on Line 3, somessage(Line 20) is not reliably announced when the trigger receives focus.♿ Proposed fix
import { computed, useId } from 'vue'; import { ExclamationTriangleIcon } from '@heroicons/vue/24/solid'; @@ -const anchor = `--ea-anchor-${useId()}`; +const id = useId(); +const anchor = `--ea-anchor-${id}`; +const tooltipId = `ea-tooltip-${id}`; const isError = computed(() => props.type === 'error');<button type="button" class="inline-flex items-center gap-2 rounded-full p-2 text-xs font-medium cursor-default ring-1 outline-none focus-visible:ring-2" :class="isError ? 'bg-red-100 ring-red-300/70 text-red-800' : 'bg-yellow-50 ring-yellow-300/70 text-yellow-800'" :style="{ anchorName: anchor }" :aria-label="title" + :aria-describedby="tooltipId" > @@ <div + :id="tooltipId" class="tooltip-panel fixed mb-2 z-20 px-2 py-1 rounded shadow-sm border text-xs hyphens-auto invisible opacity-0 transition-[opacity,visibility] duration-150 group-hover:visible group-hover:opacity-100 group-has-focus-visible:visible group-has-focus-visible:opacity-100 pointer-coarse:group-focus-within:visible pointer-coarse:group-focus-within:opacity-100"Also applies to: 26-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/emergencyaccess/EmergencyBadge.vue` around lines 3 - 20, The tooltip (the div with class "tooltip-panel" and role="tooltip") is not associated with its trigger button so screen readers won't announce message; generate a stable id (e.g., computed property tooltipId using the existing anchor prop or a UID) and add that id to the tooltip div and set the button's aria-describedby (or aria-labelledby if you prefer the title element) to the same tooltipId; ensure the binding uses :id="tooltipId" on the tooltip-panel and :aria-describedby="tooltipId" on the button and keep visibility logic unchanged so assistive tech can reference the tooltip content reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@frontend/src/components/emergencyaccess/EmergencyBadge.vue`:
- Around line 3-20: The tooltip (the div with class "tooltip-panel" and
role="tooltip") is not associated with its trigger button so screen readers
won't announce message; generate a stable id (e.g., computed property tooltipId
using the existing anchor prop or a UID) and add that id to the tooltip div and
set the button's aria-describedby (or aria-labelledby if you prefer the title
element) to the same tooltipId; ensure the binding uses :id="tooltipId" on the
tooltip-panel and :aria-describedby="tooltipId" on the button and keep
visibility logic unchanged so assistive tech can reference the tooltip content
reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0f1944a-a090-4c3a-935e-1e83e33c663d
📒 Files selected for processing (1)
frontend/src/components/emergencyaccess/EmergencyBadge.vue
overheadhunter
approved these changes
Apr 29, 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
2 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.
Summary
warning/error, dropped thepositionprop.VaultList.vueandEmergencyAccessVaultList.vue.