Skip to content

Commit

Permalink
[Bug] Fix issue where tooltip is only not shown for first item in nav…
Browse files Browse the repository at this point in the history
…list when not truncated (primer#2729)

Co-authored-by: thesnowrose <thesnowrose@users.noreply.github.com>
  • Loading branch information
thesnowrose and thesnowrose authored Mar 26, 2024
1 parent e77a77a commit 2b0a7bf
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/healthy-colts-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": minor
---

Fixes a bug where a tooltip was being shown in the navlist even when the text wasn't truncated for certain items.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
21 changes: 12 additions & 9 deletions app/components/primer/alpha/action_list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,21 @@ export class ActionListTruncationObserver {
}

update(el: HTMLElement) {
const label = el.querySelector('.ActionListItem-label')
if (!label) return
const items = el.querySelectorAll('li')

const tooltip = el.querySelector('.ActionListItem-truncationTooltip') as HTMLElement | null
if (!tooltip) return
for (const item of items) {
const label = item.querySelector('.ActionListItem-label') as HTMLElement
if (!label) continue
const tooltip = item.querySelector('.ActionListItem-truncationTooltip') as HTMLElement
if (!tooltip) continue

const isTruncated = label.scrollWidth > label.clientWidth
const isTruncated = label.scrollWidth > label.clientWidth

if (isTruncated) {
tooltip.style.display = ''
} else {
tooltip.style.display = 'none'
if (isTruncated) {
tooltip.style.display = ''
} else {
tooltip.style.display = 'none'
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion app/components/primer/alpha/action_list/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ def before_render
)

if @truncate_label == :show_tooltip && !tooltip?
with_tooltip(text: @label)
with_tooltip(text: @label, direction: :ne)
end

return unless leading_visual
Expand Down
20 changes: 19 additions & 1 deletion previews/primer/alpha/action_list_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,18 @@ def long_label_with_tooltip(truncate_label: :show_tooltip)
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
)
component.with_item(
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
)
component.with_item(
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
)
component.with_item(
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
)
end
end

Expand Down Expand Up @@ -499,8 +511,14 @@ def long_label_show_tooltip_with_truncate_label(truncate_label: :none)
) do |item|
item.with_tooltip(text: "this is a tooltip")
end
component.with_item(
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
) do |item|
item.with_tooltip(text: "this is a tooltip")
end
end
end
end
end
end
end
12 changes: 9 additions & 3 deletions previews/primer/beta/nav_list_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,15 @@ def long_label_with_tooltip(truncate_label: :show_tooltip)
component.with_item(
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
) do |item|
item.with_trailing_visual_icon(icon: :plus)
end
)
component.with_item(
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
)
component.with_item(
label: "Really really long label that may wrap, truncate, or appear as a tooltip",
truncate_label: truncate_label
)
end
end

Expand Down
2 changes: 1 addition & 1 deletion test/system/alpha/action_list_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def test_js_truncate_label_shows_tooltip
assert_selector "li.ActionListItem span.ActionListItem-label--truncate"
assert_selector "tool-tip", text: "Really really long label that may wrap, truncate, or appear as a tooltip", visible: :hidden

find("button").send_keys("tab") # sends focus to button
first("button").send_keys("tab") # sends focus to button
assert_selector "tool-tip", text: "Really really long label that may wrap, truncate, or appear as a tooltip", visible: :visible
end

Expand Down

0 comments on commit 2b0a7bf

Please sign in to comment.