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/.changeset/soft-singers-travel.md b/.changeset/soft-singers-travel.md new file mode 100644 index 0000000000..7dd8d423a9 --- /dev/null +++ b/.changeset/soft-singers-travel.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': minor +--- + +Add missing 64px Avatar size diff --git a/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/avatar/size_64/default.png b/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/avatar/size_64/default.png new file mode 100644 index 0000000000..697f910cd8 Binary files /dev/null and b/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/avatar/size_64/default.png differ diff --git a/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/avatar/size_64/focused.png b/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/avatar/size_64/focused.png new file mode 100644 index 0000000000..697f910cd8 Binary files /dev/null and b/.playwright/screenshots/snapshots.test.ts-snapshots/primer/beta/avatar/size_64/focused.png differ 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/app/components/primer/beta/avatar.rb b/app/components/primer/beta/avatar.rb index 927d87e193..e3dbafc292 100644 --- a/app/components/primer/beta/avatar.rb +++ b/app/components/primer/beta/avatar.rb @@ -25,7 +25,7 @@ class Avatar < Primer::Component DEFAULT_SHAPE = :circle SHAPE_OPTIONS = [DEFAULT_SHAPE, :square].freeze - SIZE_OPTIONS = [16, DEFAULT_SIZE, SMALL_THRESHOLD, 32, 40, 48, 80].freeze + SIZE_OPTIONS = [16, DEFAULT_SIZE, SMALL_THRESHOLD, 32, 40, 48, 64, 80].freeze # @param src [String] The source url of the avatar image. # @param alt [String] Passed through to alt on img tag. diff --git a/previews/primer/beta/avatar_preview.rb b/previews/primer/beta/avatar_preview.rb index 4cc3552e4d..ad8f25d8d3 100644 --- a/previews/primer/beta/avatar_preview.rb +++ b/previews/primer/beta/avatar_preview.rb @@ -64,6 +64,12 @@ def size_48 render(Primer::Beta::Avatar.new(size: 48, src: Primer::ExampleImage::BASE64_SRC, alt: "@kittenuser")) end + # @label 64px + # @snapshot + def size_64 + render(Primer::Beta::Avatar.new(size: 64, src: Primer::ExampleImage::BASE64_SRC, alt: "@kittenuser")) + end + # @label 80px # @snapshot def size_80 diff --git a/static/arguments.json b/static/arguments.json index 4e8cb62588..b41423ce96 100644 --- a/static/arguments.json +++ b/static/arguments.json @@ -3034,7 +3034,7 @@ "name": "size", "type": "Integer", "default": "`20`", - "description": "One of `16`, `20`, `24`, `32`, `40`, `48`, or `80`." + "description": "One of `16`, `20`, `24`, `32`, `40`, `48`, `64`, or `80`." }, { "name": "shape", diff --git a/static/constants.json b/static/constants.json index 03c803dce4..74c8ba86d7 100644 --- a/static/constants.json +++ b/static/constants.json @@ -709,6 +709,7 @@ 32, 40, 48, + 64, 80 ], "SMALL_THRESHOLD": 24 diff --git a/static/info_arch.json b/static/info_arch.json index ce5cbd0e0f..5b7680b263 100644 --- a/static/info_arch.json +++ b/static/info_arch.json @@ -9258,7 +9258,7 @@ "name": "size", "type": "Integer", "default": "`20`", - "description": "One of `16`, `20`, `24`, `32`, `40`, `48`, or `80`." + "description": "One of `16`, `20`, `24`, `32`, `40`, `48`, `64`, or `80`." }, { "name": "shape", @@ -9403,6 +9403,19 @@ ] } }, + { + "preview_path": "primer/beta/avatar/size_64", + "name": "size_64", + "snapshot": "true", + "skip_rules": { + "wont_fix": [ + "region" + ], + "will_fix": [ + "color-contrast" + ] + } + }, { "preview_path": "primer/beta/avatar/size_80", "name": "size_80", diff --git a/static/previews.json b/static/previews.json index f699a8f082..1d02ff2452 100644 --- a/static/previews.json +++ b/static/previews.json @@ -1176,6 +1176,19 @@ ] } }, + { + "preview_path": "primer/beta/avatar/size_64", + "name": "size_64", + "snapshot": "true", + "skip_rules": { + "wont_fix": [ + "region" + ], + "will_fix": [ + "color-contrast" + ] + } + }, { "preview_path": "primer/beta/avatar/size_80", "name": "size_80", 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