Skip to content

fix(ui): use DS::Tooltip and DS::Pill for the insights nav badge #2805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tryeverything24 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(ui): use DS::Tooltip and DS::Pill for the insights nav badge #2805

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions app/views/layouts/application.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,16 @@ end %>
response served on click would never clear the badge. %>
<%= link_to insights_path,
class: "relative inline-flex items-center justify-center w-9 h-9 pointer-coarse:w-11 pointer-coarse:h-11 rounded-lg text-secondary hover:text-primary hover:bg-container-inset-hover transition-colors focus-ring",
title: t("layouts.application.insights"),
aria: { label: t("layouts.application.insights") },
data: { turbo_prefetch: false } do %>
<%= icon("lightbulb", size: "sm") %>
<%# `as: :span` because the trigger sits inside this already-
focusable link — the tooltip replaces the raw title=
attribute; the link keeps its aria-label. %>
<%= render DS::Tooltip.new(text: t("layouts.application.insights"), icon: "lightbulb", size: "sm", as: :span) %>

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind the tooltip trigger to the full link

When the pointer is over the link's padding—or the link receives keyboard focus—this tooltip never opens because DS::Tooltip listens on its own descendant span; focus events do not propagate down from the <a>, and its controller only binds ancestor listeners for <summary> elements (app/components/DS/tooltip_controller.js:29-54). Consequently, replacing the link's title limits the label to hovering directly over the small lightbulb glyph instead of the full 36/44px hit target and provides no focus-triggered tooltip. The tooltip trigger/controller needs to encompass or explicitly bind to the link.

Useful? React with 👍 / 👎.

<% if unread_insights_count.positive? %>
<span class="absolute top-1 right-1 min-w-4 h-4 px-1 rounded-full bg-inverse text-inverse text-[10px] font-medium flex items-center justify-center pointer-events-none"><%= unread_insights_count > 9 ? "9+" : unread_insights_count %></span>
<span class="absolute -top-1 -right-1 pointer-events-none">
<%= render DS::Pill.new(label: unread_insights_count > 9 ? "9+" : unread_insights_count.to_s, tone: :neutral, style: :filled, marker: false) %>
</span>
<% end %>
<% end %>
<% end %>
Expand Down
Loading