Skip to content

Commit 711ddc4

Browse files
fix(tooltip): non-focusable trigger slot for composition (#62)
1 parent f2b6226 commit 711ddc4

14 files changed

Lines changed: 276 additions & 60 deletions

File tree

assets/components/tooltip.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,12 @@ export class Tooltip extends Component<Props, Api> {
2727
triggerEls.forEach((triggerEl) => {
2828
const raw = triggerEl.dataset.value;
2929
const valueProps = raw != null && raw !== "" ? { value: raw } : {};
30-
this.spreadProps(triggerEl, this.api.getTriggerProps(valueProps));
30+
const tabIndexOverride = triggerEl.getAttribute("tabindex");
31+
const triggerProps = this.api.getTriggerProps(valueProps);
32+
if (tabIndexOverride === "-1") {
33+
triggerProps.tabIndex = -1;
34+
}
35+
this.spreadProps(triggerEl, triggerProps);
3136
});
3237

3338
const positionerEl = rootEl.querySelector<HTMLElement>(

assets/test/component/tooltip.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,15 @@ describe("Tooltip", () => {
1111
expect(el.querySelector('[data-part="positioner"]')).toBeTruthy();
1212
c.destroy();
1313
});
14+
15+
it("preserves tabindex -1 on trigger after render", () => {
16+
const el = tooltipTree();
17+
const trigger = el.querySelector<HTMLElement>('[data-part="trigger"]');
18+
expect(trigger).toBeTruthy();
19+
trigger!.setAttribute("tabindex", "-1");
20+
const c = new Tooltip(el, { id: el.id });
21+
c.render();
22+
expect(trigger!.getAttribute("tabindex")).toBe("-1");
23+
c.destroy();
24+
});
1425
});

e2e/lib/e2e_web/demos/tooltip_demo.ex

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,49 @@ defmodule E2eWeb.Demos.TooltipDemo do
273273
"""
274274
end
275275

276+
def patterns_menu_item_items do
277+
Corex.Tree.new([
278+
%{label: "Edit", value: "edit"},
279+
%{label: "Support", value: "support", disabled: true}
280+
])
281+
end
282+
283+
def patterns_menu_item_heex do
284+
~S"""
285+
<.menu id="tooltip-pattern-menu" class="menu" items={@items}>
286+
<:trigger>Actions</:trigger>
287+
<:indicator>
288+
<.heroicon name="hero-chevron-down" />
289+
</:indicator>
290+
<:item :let={item}>
291+
<%= if item.value == "support" do %>
292+
<.tooltip
293+
id="tooltip-pattern-menu-support"
294+
class="tooltip tooltip--sm"
295+
trigger_tag={:span}
296+
show_arrow={false}
297+
>
298+
<:trigger focusable={false}>{item.label}</:trigger>
299+
<:content>Coming soon</:content>
300+
</.tooltip>
301+
<% else %>
302+
{item.label}
303+
<% end %>
304+
</:item>
305+
</.menu>
306+
"""
307+
end
308+
309+
def patterns_menu_item_elixir do
310+
~S"""
311+
items =
312+
Corex.Tree.new([
313+
%{label: "Edit", value: "edit"},
314+
%{label: "Support", value: "support", disabled: true}
315+
])
316+
"""
317+
end
318+
276319
def api_set_open_client_binding_heex do
277320
~S"""
278321
<div class="layout__row">

e2e/lib/e2e_web/live/pages_live/tooltip_patterns_live.ex

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ defmodule E2eWeb.TooltipPatternsLive do
6262
path={@path}
6363
id="tooltip-patterns-page"
6464
title="Tooltip · Pattern"
65-
subtitle="Multi-trigger with LiveView, per-row tooltips in a ul, and one tooltip with several link triggers."
65+
subtitle="Multi-trigger with LiveView, per-row tooltips in a ul, link triggers, and tooltip inside a menu item."
6666
>
6767
<.demo_section
6868
id="tooltip-pattern-multi-trigger-lv"
@@ -181,6 +181,53 @@ defmodule E2eWeb.TooltipPatternsLive do
181181
</div>
182182
</:preview>
183183
</.demo_section>
184+
185+
<.demo_section
186+
id="tooltip-pattern-menu-item"
187+
title="Tooltip inside menu item"
188+
code_tabs={[
189+
%{
190+
value: "heex",
191+
label: "Heex",
192+
language: :heex,
193+
code: E2eWeb.Demos.TooltipDemo.patterns_menu_item_heex()
194+
},
195+
%{
196+
value: "elixir",
197+
label: ~t"Elixir",
198+
language: :elixir,
199+
code: E2eWeb.Demos.TooltipDemo.patterns_menu_item_elixir()
200+
}
201+
]}
202+
>
203+
<:preview>
204+
<.menu
205+
id="tooltip-pattern-menu"
206+
class="menu"
207+
items={E2eWeb.Demos.TooltipDemo.patterns_menu_item_items()}
208+
>
209+
<:trigger>Actions</:trigger>
210+
<:indicator>
211+
<.heroicon name="hero-chevron-down" />
212+
</:indicator>
213+
<:item :let={item}>
214+
<%= if item.value == "support" do %>
215+
<.tooltip
216+
id="tooltip-pattern-menu-support"
217+
class="tooltip tooltip--sm"
218+
trigger_tag={:span}
219+
show_arrow={false}
220+
>
221+
<:trigger focusable={false}>{item.label}</:trigger>
222+
<:content>Coming soon</:content>
223+
</.tooltip>
224+
<% else %>
225+
{item.label}
226+
<% end %>
227+
</:item>
228+
</.menu>
229+
</:preview>
230+
</.demo_section>
184231
</.demo_page>
185232
</Layouts.app>
186233
"""

e2e/test/components/tooltip_markup_test.exs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,11 @@ defmodule E2eWeb.TooltipMarkupTest do
4747
assert html =~ ~S(id="tooltip-pattern-profile-link-multi-tool")
4848
assert html =~ ~S(data-on-trigger-value-change="tooltip_pattern_link_multi_value")
4949
assert html =~ ~S(id="tooltip:tooltip-pattern-profile-link-multi-tool:trigger:1")
50+
assert html =~ "tooltip-pattern-menu-item"
51+
assert html =~ ~S(id="menu:tooltip-pattern-menu")
52+
assert html =~ ~S(id="tooltip-pattern-menu-support")
53+
assert html =~ ~S(id="tooltip:tooltip-pattern-menu-support:trigger")
54+
assert html =~ ~S|id="tooltip:tooltip-pattern-menu-support:trigger" tabindex="-1"|
5055
end
5156

5257
test "renders arrow parts when show_arrow", %{conn: conn} do

e2e/test/components/tooltip_test.exs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ defmodule E2eWeb.TooltipTest do
55
import Wallaby.Query
66

77
alias E2eWeb.ComponentBehaviorSpec
8+
alias E2eWeb.MenuModel, as: Menu
89
alias E2eWeb.TooltipModel, as: Tooltip
910

1011
@moduletag :tooltip
@@ -101,5 +102,26 @@ defmodule E2eWeb.TooltipTest do
101102
|> Tooltip.open_first_tooltip_in_section(section)
102103
|> Tooltip.wait_open_content_in_section(section, timeout: 8_000)
103104
end
105+
106+
feature "menu item - open menu does not open tooltip; hover opens tooltip", %{
107+
session: session
108+
} do
109+
section = "tooltip-pattern-menu-item"
110+
menu_host = "tooltip-pattern-menu"
111+
112+
session =
113+
session
114+
|> ComponentBehaviorSpec.visit_ready(Tooltip, :tooltip, :patterns)
115+
|> Tooltip.wait_patterns_page()
116+
|> Menu.wait_section_menu_ready(section)
117+
|> Tooltip.wait_section_tooltip_ready(section)
118+
|> Menu.open_menu_in_section(section)
119+
|> Menu.wait_menu_content_open(menu_host, timeout: 8_000)
120+
|> Tooltip.refute_open_content_in_section(section)
121+
122+
session
123+
|> Tooltip.hover_first_trigger_in_section(section)
124+
|> Tooltip.wait_open_content_in_section(section, timeout: 8_000)
125+
end
104126
end
105127
end

e2e/test/support/models/tooltip_model.ex

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,13 @@ defmodule E2eWeb.TooltipModel do
9191
def hover_first_trigger_in_section(session, section_dom_id) do
9292
if not valid_dom_id?(section_dom_id), do: raise(ArgumentError, "invalid section dom id")
9393

94-
_ =
95-
execute_script(
96-
session,
97-
"""
98-
const section = document.querySelector(arguments[0]);
99-
if (!section) return;
100-
const trigger = section.querySelector('[data-scope="tooltip"][data-part="trigger"]');
101-
if (!trigger) return;
102-
trigger.scrollIntoView({block: 'center'});
103-
trigger.dispatchEvent(new MouseEvent('pointerenter', { bubbles: true }));
104-
trigger.dispatchEvent(new MouseEvent('mouseover', { bubbles: true }));
105-
""",
106-
["section#" <> section_dom_id]
94+
hover(
95+
session,
96+
css(
97+
~s|section##{section_dom_id} [data-scope="tooltip"][data-part="trigger"]|,
98+
visible: :any
10799
)
100+
)
108101

109102
session
110103
end
@@ -122,6 +115,20 @@ defmodule E2eWeb.TooltipModel do
122115
session
123116
end
124117

118+
def refute_open_content_in_section(session, section_dom_id) do
119+
if not valid_dom_id?(section_dom_id), do: raise(ArgumentError, "invalid section dom id")
120+
121+
refute_has(
122+
session,
123+
css(
124+
~s|section##{section_dom_id} [data-scope="tooltip"][data-part="content"][data-state="open"]|,
125+
visible: :any
126+
)
127+
)
128+
129+
session
130+
end
131+
125132
def wait_open_content_in_host(session, host_dom_id, opts \\ []) do
126133
wait_for_has(
127134
session,

lib/components/tooltip.ex

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,21 @@ defmodule Corex.Tooltip do
106106
end
107107
```
108108
109+
### Inside menu (or other roving-focus containers)
110+
111+
When a tooltip trigger sits inside a menu item, set `focusable={false}` on `<:trigger>` so
112+
open focus does not land on the trigger and open the tooltip. Prefer `trigger_tag={:span}`
113+
to avoid nested `<button>` elements inside `role="menuitem"`.
114+
115+
```heex
116+
<.menu_item disabled value="support">
117+
<.tooltip class="tooltip tooltip--sm" trigger_tag={:span}>
118+
<:trigger focusable={false}>Support</:trigger>
119+
<:content>Coming soon</:content>
120+
</.tooltip>
121+
</.menu_item>
122+
```
123+
109124
<!-- tabs-close -->
110125
111126
## Style
@@ -271,6 +286,16 @@ defmodule Corex.Tooltip do
271286
"Trigger element content. Use :let={tooltip} for assigns such as disabled. With more than one <:trigger>, each must set value=\"\" (unique, stable across LiveView patches)." do
272287
attr(:class, :string, required: false)
273288
attr(:value, :string, required: false)
289+
290+
attr(:focusable, :boolean,
291+
doc:
292+
"When false, the trigger is not tabbable (tabindex -1). Use inside menus or other roving-focus containers so focus does not open the tooltip. Defaults to true."
293+
)
294+
295+
attr(:tabindex, :integer,
296+
doc:
297+
"Explicit tabindex for the trigger. Overrides focusable when set. Tooltip disabled still forces -1."
298+
)
274299
end
275300

276301
slot :content,
@@ -317,52 +342,16 @@ defmodule Corex.Tooltip do
317342
<%= if @trigger_tag == :span do %>
318343
<span
319344
class={Map.get(t, :class, nil)}
320-
phx-mounted={
321-
Connect.ignore_trigger(%Trigger{
322-
id: @id,
323-
dir: @dir,
324-
open: false,
325-
disabled: @disabled,
326-
orientation: @orientation,
327-
tag: @trigger_tag,
328-
value: Map.get(t, :value)
329-
})
330-
}
331-
{Connect.trigger(%Trigger{
332-
id: @id,
333-
dir: @dir,
334-
open: false,
335-
disabled: @disabled,
336-
orientation: @orientation,
337-
tag: @trigger_tag,
338-
value: Map.get(t, :value)
339-
})}
345+
phx-mounted={Connect.ignore_trigger(trigger_connect_assigns(@id, @dir, @orientation, @disabled, @trigger_tag, t))}
346+
{Connect.trigger(trigger_connect_assigns(@id, @dir, @orientation, @disabled, @trigger_tag, t))}
340347
>
341348
{render_slot(t, @slot_assigns)}
342349
</span>
343350
<% else %>
344351
<button
345352
class={Map.get(t, :class, nil)}
346-
phx-mounted={
347-
Connect.ignore_trigger(%Trigger{
348-
id: @id,
349-
dir: @dir,
350-
open: false,
351-
disabled: @disabled,
352-
orientation: @orientation,
353-
tag: @trigger_tag,
354-
value: Map.get(t, :value)
355-
})
356-
}
357-
{Connect.trigger(%Trigger{
358-
id: @id,
359-
dir: @dir,
360-
open: false,
361-
disabled: @disabled,
362-
orientation: @orientation,
363-
tag: @trigger_tag,
364-
value: Map.get(t, :value)
365-
})}
353+
phx-mounted={Connect.ignore_trigger(trigger_connect_assigns(@id, @dir, @orientation, @disabled, @trigger_tag, t))}
354+
{Connect.trigger(trigger_connect_assigns(@id, @dir, @orientation, @disabled, @trigger_tag, t))}
366355
>
367356
{render_slot(t, @slot_assigns)}
368357
</button>
@@ -445,6 +434,20 @@ defmodule Corex.Tooltip do
445434
})
446435
end
447436

437+
defp trigger_connect_assigns(id, dir, orientation, disabled, trigger_tag, trigger_slot) do
438+
%Trigger{
439+
id: id,
440+
dir: dir,
441+
open: false,
442+
disabled: disabled,
443+
orientation: orientation,
444+
tag: trigger_tag,
445+
value: Map.get(trigger_slot, :value),
446+
focusable: Map.get(trigger_slot, :focusable, true),
447+
tabindex: Map.get(trigger_slot, :tabindex)
448+
}
449+
end
450+
448451
defp validate_triggers!(triggers) when is_list(triggers) do
449452
case triggers do
450453
[] ->

lib/components/tooltip/anatomy.ex

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,17 @@ defmodule Corex.Tooltip.Anatomy do
4646

4747
defmodule Trigger do
4848
@moduledoc false
49-
defstruct [:id, :dir, :open, :disabled, orientation: "horizontal", tag: :button, value: nil]
49+
defstruct [
50+
:id,
51+
:dir,
52+
:open,
53+
:disabled,
54+
:tabindex,
55+
orientation: "horizontal",
56+
tag: :button,
57+
value: nil,
58+
focusable: true
59+
]
5060

5161
@ignored_attrs [
5262
"type",

lib/components/tooltip/connect.ex

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ defmodule Corex.Tooltip.Connect do
4949
base = %{
5050
"data-scope" => "tooltip",
5151
"data-part" => "trigger",
52-
"tabindex" => if(assigns.disabled, do: -1, else: 0),
52+
"tabindex" => trigger_tabindex(assigns),
5353
"data-disabled" => assigns.disabled,
5454
"dir" => Map.get(assigns, :dir),
5555
"data-orientation" => Map.get(assigns, :orientation, "vertical"),
@@ -144,4 +144,13 @@ defmodule Corex.Tooltip.Connect do
144144
to: Selectors.css_id("tooltip:#{assigns.id}:arrow-tip")
145145
)
146146
end
147+
148+
defp trigger_tabindex(assigns) do
149+
cond do
150+
assigns.disabled -> -1
151+
is_integer(Map.get(assigns, :tabindex)) -> Map.get(assigns, :tabindex)
152+
Map.get(assigns, :focusable, true) == false -> -1
153+
true -> 0
154+
end
155+
end
147156
end

0 commit comments

Comments
 (0)