Skip to content

Commit 326c91d

Browse files
committed
Change the param name and fix the misalignment between text and chavron
1 parent 14e8169 commit 326c91d

9 files changed

Lines changed: 40 additions & 9 deletions

File tree

.changeset/calm-trees-align.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@openproject/primer-view-components': patch
3+
---
4+
5+
Align asynchronously loaded single-select TreeView labels with their toggles

app/components/primer/alpha/tree_view.pcss

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,13 @@
288288
height: var(--custom-line-height, 1.3rem);
289289
color: var(--fgColor-muted);
290290
align-items: center;
291+
292+
/* Unlike leading visuals, the single-select checkmark should align with the first line's box instead of
293+
participating in baseline alignment. A second baseline-aligned visual changes the row height and causes the
294+
node label to appear lower than its toggle. */
295+
&.TreeViewItem-singleSelectCheckmark {
296+
align-self: flex-start;
297+
}
291298
}
292299

293300
& .TreeViewItemLeadingAction {

app/components/primer/alpha/tree_view/node.html.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
<% end %>
2525
<span class="TreeViewItemContentText"><%= text_content %></span>
2626
<% if @select_variant == :single %>
27-
<span class="TreeViewItemVisual" aria-hidden="true">
28-
<%= render(Primer::Beta::Octicon.new(icon: :check, classes: "TreeViewItem-singleSelectCheckmark")) %>
27+
<span class="TreeViewItemVisual TreeViewItem-singleSelectCheckmark" aria-hidden="true">
28+
<%= render(Primer::Beta::Octicon.new(icon: :check)) %>
2929
</span>
3030
<% elsif trailing_visual? %>
3131
<%= trailing_visual %>

app/controllers/primer/view_components/tree_view_items_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def async_alpha
4646

4747
render(
4848
locals: {
49-
action_menu_expanded: params[:action_menu_expanded] == "true",
49+
nested_child_expanded: params[:nested_child_expanded] == "true",
5050
select_variant: (params[:select_variant] || :none).to_sym
5151
}
5252
)

app/views/primer/view_components/tree_view_items/async_alpha.html_fragment.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<% icons.with_collapsed_icon(icon: :"file-directory-fill", color: :accent) %>
66
<% end %>
77

8-
<% sub_tree.with_sub_tree(label: "action_menu", expanded: action_menu_expanded, select_variant: select_variant, select_strategy: :self) do |sub_tree| %>
8+
<% sub_tree.with_sub_tree(label: "action_menu", expanded: nested_child_expanded, select_variant: select_variant, select_strategy: :self) do |sub_tree| %>
99
<% sub_tree.with_leading_visual_icons do |icons| %>
1010
<% icons.with_expanded_icon(icon: :"file-directory-open-fill", color: :accent) %>
1111
<% icons.with_collapsed_icon(icon: :"file-directory-fill", color: :accent) %>

previews/primer/alpha/tree_view_preview.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,11 @@ def loading_skeleton(simulate_failure: false, simulate_empty: false)
113113

114114
# @label Async alpha
115115
#
116-
# @param action_menu_expanded [Boolean] toggle
116+
# @param nested_child_expanded [Boolean] toggle
117117
# @param select_variant [Symbol] select [none, single, multiple]
118-
def async_alpha(action_menu_expanded: false, select_variant: :none)
118+
def async_alpha(nested_child_expanded: false, select_variant: :none)
119119
render_with_template(locals: {
120-
action_menu_expanded: coerce_bool(action_menu_expanded),
120+
nested_child_expanded: coerce_bool(nested_child_expanded),
121121
select_variant: select_variant.to_sym
122122
})
123123
end

previews/primer/alpha/tree_view_preview/async_alpha.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<% icons.with_collapsed_icon(icon: :"file-directory-fill", color: :accent) %>
77
<% end %>
88

9-
<% sub_tree.with_loading_skeleton(src: primer_view_components.tree_view_items_async_alpha_path(action_menu_expanded: action_menu_expanded, select_variant: select_variant)) %>
9+
<% sub_tree.with_loading_skeleton(src: primer_view_components.tree_view_items_async_alpha_path(nested_child_expanded: nested_child_expanded, select_variant: select_variant)) %>
1010
<% end %>
1111
<% end %>
1212
</div>

test/components/primer/alpha/tree_view_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,16 @@ def test_disallows_trailing_visuals_when_single_select_variant_is_used
259259

260260
assert_equal error.message, "Trailing visuals can't be used in combination with single select mode as the icon is reserved."
261261
end
262+
263+
def test_single_select_checkmark_is_the_trailing_visual
264+
render_inline(Primer::Alpha::TreeView.new) do |tree|
265+
tree.with_leaf(label: "src", select_variant: :single)
266+
end
267+
268+
assert_selector ".TreeViewItemContent > .TreeViewItemVisual.TreeViewItem-singleSelectCheckmark" do |visual|
269+
visual.assert_selector "svg.octicon-check"
270+
end
271+
end
262272
end
263273
end
264274
end

test/system/alpha/tree_view_test.rb

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ def test_automatically_expands_all_ancestors
117117

118118
# This explicitly tests the MutationObserver in tree_view.ts that listens for expanded nodes
119119
def test_automatically_expands_all_ancestors_when_async_items_have_expanded_child
120-
visit_preview(:async_alpha, action_menu_expanded: true)
120+
visit_preview(:async_alpha, nested_child_expanded: true)
121121

122122
activate_at_path("primer")
123123
assert node_at_path("primer", "alpha", "action_menu", "heading.rb")
@@ -735,6 +735,15 @@ def test_single_select_works_after_async_loading
735735
assert_path("primer", "alpha")
736736
assert_path_checked("primer")
737737

738+
label_and_toggle_centers = evaluate_multiline_script(<<~JS)
739+
const node = document.querySelector(#{selector_for("primer", "alpha").to_json})
740+
const labelRect = node.querySelector('.TreeViewItemContentText').getBoundingClientRect()
741+
const toggleRect = node.parentElement.querySelector('.TreeViewItemToggle svg:not([hidden])').getBoundingClientRect()
742+
743+
return [labelRect.y + labelRect.height / 2, toggleRect.y + toggleRect.height / 2]
744+
JS
745+
assert_in_delta label_and_toggle_centers[0], label_and_toggle_centers[1], 0.5
746+
738747
# Expand down to the leaf nodes
739748
expand_at_path("primer", "alpha")
740749
expand_at_path("primer", "alpha", "action_menu")

0 commit comments

Comments
 (0)