From fb2208c196e8d2afd626cc2887392352a46fd2b4 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Wed, 2 Oct 2024 10:35:49 -0700 Subject: [PATCH] Resolve TODOs --- app/components/primer/alpha/stack.rb | 9 +++++ app/components/primer/alpha/stack_item.rb | 2 + app/components/primer/responsive_arg.rb | 1 - test/components/alpha/stack_item_test.rb | 47 ++++++++++++++--------- test/components/alpha/stack_test.rb | 23 +++++------ 5 files changed, 49 insertions(+), 33 deletions(-) diff --git a/app/components/primer/alpha/stack.rb b/app/components/primer/alpha/stack.rb index 43ad982647..91150f62a4 100644 --- a/app/components/primer/alpha/stack.rb +++ b/app/components/primer/alpha/stack.rb @@ -127,6 +127,15 @@ def self.arg_name end end + ARG_CLASSES = [ + JustifyArg, + DirectionArg, + AlignArg, + WrapArg, + PaddingArg, + GapArg + ].freeze + # @param tag [Symbol] Customize the element type of the rendered container. # @param gap [Symbol] Specify the gap between children elements in the stack. <%= one_of(Primer::Alpha::Stack::GapArg::OPTIONS) %> diff --git a/app/components/primer/alpha/stack_item.rb b/app/components/primer/alpha/stack_item.rb index 10134aefa8..be5f0e6109 100644 --- a/app/components/primer/alpha/stack_item.rb +++ b/app/components/primer/alpha/stack_item.rb @@ -24,6 +24,8 @@ def self.arg_name end end + ARG_CLASSES = [GrowArg].freeze + # @param tag [Symbol] Customize the element type of the rendered container. # @param grow [Boolean] Allow item to keep size or expand to fill the available space. # @param system_arguments [Hash] <%= link_to_system_arguments_docs %> diff --git a/app/components/primer/responsive_arg.rb b/app/components/primer/responsive_arg.rb index dd71453b1c..19c553a0bc 100644 --- a/app/components/primer/responsive_arg.rb +++ b/app/components/primer/responsive_arg.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# TODO: should this "live" here? module Primer # Base class for responsive Stack and StackItem arguments. Used internally. class ResponsiveArg diff --git a/test/components/alpha/stack_item_test.rb b/test/components/alpha/stack_item_test.rb index 2da05374d0..6faff914d0 100644 --- a/test/components/alpha/stack_item_test.rb +++ b/test/components/alpha/stack_item_test.rb @@ -37,7 +37,7 @@ def test_allows_customizing_tag assert_selector("a.StackItem") end - def test_renders_static_prop_grow_with_true_option + def test_renders_static_prop_grow_with_true_option render_inline(Primer::Alpha::StackItem.new(grow: true)) do "content" end @@ -45,36 +45,35 @@ def test_renders_static_prop_grow_with_true_option assert_selector("div[data-grow=\"true\"]") end - def test_ignores_static_prop_grow_with_false_option + def test_ignores_static_prop_grow_with_false_option render_inline(Primer::Alpha::StackItem.new(grow: false)) do "content" end - assert_selector("div:not([data-grow])") + refute_selector("div[data-grow]") end def test_ignores_static_prop_grow_by_default - render_inline(Primer::Alpha::StackItem.new()) do + render_inline(Primer::Alpha::StackItem.new) do "content" end - assert_selector("div:not([data-grow])") + refute_selector("div[data-grow]") end - def test_ignores_responsive_prop_grow_with_false_option + def test_ignores_responsive_prop_grow_with_false_option render_inline(Primer::Alpha::StackItem.new(grow: [false]*5)) do "content" end - assert_selector("div:not([data-grow])") - assert_selector("div:not([data-grow-regular])") - assert_selector("div:not([data-grow-narrow])") - assert_selector("div:not([data-grow-wide])") + refute_selector("div[data-grow]") + refute_selector("div[data-grow-regular]") + refute_selector("div[data-grow-narrow]") + refute_selector("div[data-grow-wide]") end - # TODO: do we care that it's data-grow=true instead of just data-grow? - def test_renders_responsive_prop_grow_with_true_option - render_inline(Primer::Alpha::StackItem.new(grow: [true]*5)) do + def test_renders_responsive_prop_grow_with_true_option + render_inline(Primer::Alpha::StackItem.new(grow: [true] * 5)) do "content" end @@ -84,18 +83,28 @@ def test_renders_responsive_prop_grow_with_true_option assert_selector("div[data-grow-wide=\"true\"]") end - # TODO: consider this scenario where l and xl are two different values but both map to wide... - def test_renders_responsive_prop_grow_with_mixed_options - render_inline(Primer::Alpha::StackItem.new(grow: [true,false,false,true,false])) do + def test_renders_responsive_prop_grow_with_responsive_options + render_inline(Primer::Alpha::StackItem.new(grow: [true, false, false, true, false])) do "content" end assert_selector("div[data-grow=\"true\"]") - assert_selector("div:not([data-grow-regular])") - assert_selector("div:not([data-grow-narrow])") + refute_selector("div[data-grow-regular]") + refute_selector("div[data-grow-narrow]") + assert_selector("div[data-grow-wide=\"true\"]") + end + + def test_renders_wide_when_xl_option_provided + render_inline(Primer::Alpha::StackItem.new(grow: [false, false, false, false, true])) do + "content" + end + + refute_selector("div[data-grow]") + refute_selector("div[data-grow-regular]") + refute_selector("div[data-grow-narrow]") assert_selector("div[data-grow-wide=\"true\"]") end - + def test_status assert_component_state(Primer::Alpha::StackItem, :alpha) end diff --git a/test/components/alpha/stack_test.rb b/test/components/alpha/stack_test.rb index 477ae20254..b3952971d6 100644 --- a/test/components/alpha/stack_test.rb +++ b/test/components/alpha/stack_test.rb @@ -64,32 +64,29 @@ def test_allows_customizing_tag end # Static arg tests, i.e. Stack.new(justify: :center) - Primer::ResponsiveArg.descendants.each do |descendant| - # TODO: fix this (StackItem's arg) - next unless descendant.arg_name != :grow - descendant::OPTIONS.each do |option| + Primer::Alpha::Stack::ARG_CLASSES.each do |arg_class| + arg_class::OPTIONS.each do |option| next unless option - define_method("test_renders_static_arg_#{descendant.arg_name}_with_#{option}_option") do - render_inline(Primer::Alpha::Stack.new(descendant.arg_name => option)) do + define_method("test_renders_static_arg_#{arg_class.arg_name}_with_#{option}_option") do + render_inline(Primer::Alpha::Stack.new(arg_class.arg_name => option)) do "content" end - assert_selector(".Stack[data-#{descendant.arg_name.to_s.dasherize}=\"#{option.to_s.dasherize}\"]") + assert_selector(".Stack[data-#{arg_class.arg_name.to_s.dasherize}=\"#{option.to_s.dasherize}\"]") end end end - Primer::ResponsiveArg.descendants.each do |descendant| - # TODO: fix this (StackItem's arg) - next unless descendant.arg_name != :grow - next unless descendant::DEFAULT - define_method("test_renders_static_arg_#{descendant.arg_name}_with_default_option") do + Primer::Alpha::Stack::ARG_CLASSES.each do |arg_class| + next unless arg_class::DEFAULT + + define_method("test_renders_static_arg_#{arg_class.arg_name}_with_default_option") do render_inline(Primer::Alpha::Stack.new) do "content" end - assert_selector(".Stack[data-#{descendant.arg_name.to_s.dasherize}=\"#{descendant::DEFAULT.to_s.dasherize}\"]") + assert_selector(".Stack[data-#{arg_class.arg_name.to_s.dasherize}=\"#{arg_class::DEFAULT.to_s.dasherize}\"]") end end