From 314c0f1f1ec8152c0a0862ee3468e88efcad0ec8 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Thu, 9 Nov 2023 15:10:26 -0800 Subject: [PATCH] Remove touch target gaps between ActionBar items (#2365) --- .changeset/popular-timers-shop.md | 5 +++ app/components/primer/alpha/action_bar.pcss | 18 +------- test/components/alpha/action_bar_test.rb | 37 ++++++++++++++++ .../primer/alpha/action_bar_test.rb | 44 ------------------- 4 files changed, 43 insertions(+), 61 deletions(-) create mode 100644 .changeset/popular-timers-shop.md delete mode 100644 test/components/primer/alpha/action_bar_test.rb diff --git a/.changeset/popular-timers-shop.md b/.changeset/popular-timers-shop.md new file mode 100644 index 0000000000..6922ee5f3d --- /dev/null +++ b/.changeset/popular-timers-shop.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': minor +--- + +Remove touch target gaps between ActionBar items diff --git a/app/components/primer/alpha/action_bar.pcss b/app/components/primer/alpha/action_bar.pcss index 0680c23bda..86f11275ff 100644 --- a/app/components/primer/alpha/action_bar.pcss +++ b/app/components/primer/alpha/action_bar.pcss @@ -2,7 +2,7 @@ .ActionBar { position: relative; display: flex !important; - min-width: calc(var(--control-medium-size) * 3); + min-width: calc(var(--control-medium-size) * 3); align-items: center; flex-grow: 1; flex-shrink: 1; @@ -51,19 +51,3 @@ .ActionBar--large .ActionBar-divider { margin: 0 var(--controlStack-large-gap-condensed); } - -/* Increase spacing so touch targets don't overlap */ - -@media (pointer: coarse) { - .ActionBar .ActionBar-item-container { - gap: calc(var(--control-minTarget-coarse) - var(--control-medium-size)); /* 12px */ - } - - .ActionBar--small .ActionBar-item-container { - gap: calc(var(--control-minTarget-coarse) - var(--control-small-size)); /* 16px */ - } - - .ActionBar--large .ActionBar-item-container { - gap: calc(var(--control-minTarget-coarse) - var(--control-large-size)); /* 4px */ - } -} diff --git a/test/components/alpha/action_bar_test.rb b/test/components/alpha/action_bar_test.rb index cb409b315e..dc86bdde25 100644 --- a/test/components/alpha/action_bar_test.rb +++ b/test/components/alpha/action_bar_test.rb @@ -7,6 +7,43 @@ module Alpha class ActionBarTest < Minitest::Test include Primer::ComponentTestHelpers + def test_renders + render_inline(Primer::Alpha::ActionBar.new) do |component| + component.with_item_icon_button(icon: :pencil, label: "Button 1") + component.with_item_icon_button(icon: :pencil, label: "Button 2") + component.with_item_icon_button(icon: :pencil, label: "Button 3") + component.with_item_icon_button(icon: :pencil, label: "Button 4") + end + + assert_selector("action-bar") do + assert_selector("tool-tip", text: "Button 1") + assert_selector("tool-tip", text: "Button 2") + assert_selector("tool-tip", text: "Button 3") + assert_selector("tool-tip", text: "Button 4") + assert_selector("[data-target=\"action-bar.moreMenu\"]", visible: :hidden) + end + end + + def test_size_small + render_inline(Primer::Alpha::ActionBar.new(size: :small)) do |component| + component.with_item_icon_button(icon: :pencil, label: "Button 1") + component.with_item_icon_button(icon: :pencil, label: "Button 2") + component.with_item_divider + component.with_item_icon_button(icon: :pencil, label: "Button 3") + component.with_item_icon_button(icon: :pencil, label: "Button 4") + end + + assert_selector("[data-targets=\"action-bar.items\"] .Button--small", count: 4) + end + + def test_item_merges_item_arguments + render_inline(Primer::Alpha::ActionBar.new(size: :small)) do |component| + component.with_item_icon_button(icon: :pencil, label: "Button 1", item_arguments: { classes: "foo", tag: :span }) + end + + assert_selector("span.foo.ActionBar-item") + end + def test_renders_action_menu_items_with_type_button render_preview(:default) diff --git a/test/components/primer/alpha/action_bar_test.rb b/test/components/primer/alpha/action_bar_test.rb deleted file mode 100644 index c1a4487bfa..0000000000 --- a/test/components/primer/alpha/action_bar_test.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -require "components/test_helper" - -class PrimerAlphaActionBarTest < Minitest::Test - include Primer::ComponentTestHelpers - - def test_renders - render_inline(Primer::Alpha::ActionBar.new) do |component| - component.with_item_icon_button(icon: :pencil, label: "Button 1") - component.with_item_icon_button(icon: :pencil, label: "Button 2") - component.with_item_icon_button(icon: :pencil, label: "Button 3") - component.with_item_icon_button(icon: :pencil, label: "Button 4") - end - - assert_selector("action-bar") do - assert_selector("tool-tip", text: "Button 1") - assert_selector("tool-tip", text: "Button 2") - assert_selector("tool-tip", text: "Button 3") - assert_selector("tool-tip", text: "Button 4") - assert_selector("[data-target=\"action-bar.moreMenu\"]", visible: :hidden) - end - end - - def test_size_small - render_inline(Primer::Alpha::ActionBar.new(size: :small)) do |component| - component.with_item_icon_button(icon: :pencil, label: "Button 1") - component.with_item_icon_button(icon: :pencil, label: "Button 2") - component.with_item_divider - component.with_item_icon_button(icon: :pencil, label: "Button 3") - component.with_item_icon_button(icon: :pencil, label: "Button 4") - end - - assert_selector("[data-targets=\"action-bar.items\"] .Button--small", count: 4) - end - - def test_item_merges_item_arguments - render_inline(Primer::Alpha::ActionBar.new(size: :small)) do |component| - component.with_item_icon_button(icon: :pencil, label: "Button 1", item_arguments: { classes: "foo", tag: :span }) - end - - assert_selector("span.foo.ActionBar-item") - end -end