From 8587fc03504deeed74526fb2c9fb95ac63347d8f Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Mon, 14 Oct 2024 11:21:06 -0700 Subject: [PATCH 1/4] Fix gitignore for Primer forms move (#3147) --- .gitignore | 10 +- {lib => app/lib}/primer/forms.rb | 0 app/lib/primer/forms/primer_multi_input.d.ts | 10 -- app/lib/primer/forms/primer_text_field.d.ts | 28 ----- app/lib/primer/forms/primer_text_field.js | 119 ------------------ app/lib/primer/forms/toggle_switch_input.d.ts | 5 - package.json | 4 +- 7 files changed, 6 insertions(+), 170 deletions(-) rename {lib => app/lib}/primer/forms.rb (100%) delete mode 100644 app/lib/primer/forms/primer_multi_input.d.ts delete mode 100644 app/lib/primer/forms/primer_text_field.d.ts delete mode 100644 app/lib/primer/forms/primer_text_field.js delete mode 100644 app/lib/primer/forms/toggle_switch_input.d.ts diff --git a/.gitignore b/.gitignore index 85f5acaa6b..663581ce33 100644 --- a/.gitignore +++ b/.gitignore @@ -5,11 +5,11 @@ app/components/**/*.css.json app/components/**/*.css.map app/components/**/*.d.ts app/lib/primer/css/*.css.json -lib/primer/forms/**/*.js -lib/primer/forms/**/*.css -lib/primer/forms/**/*.css.json -lib/primer/forms/**/*.css.map -lib/primer/forms/**/*.d.ts +app/lib/primer/forms/**/*.js +app/lib/primer/forms/**/*.css +app/lib/primer/forms/**/*.css.json +app/lib/primer/forms/**/*.css.map +app/lib/primer/forms/**/*.d.ts app/assets/ # Generated by demo npm post-install diff --git a/lib/primer/forms.rb b/app/lib/primer/forms.rb similarity index 100% rename from lib/primer/forms.rb rename to app/lib/primer/forms.rb diff --git a/app/lib/primer/forms/primer_multi_input.d.ts b/app/lib/primer/forms/primer_multi_input.d.ts deleted file mode 100644 index 7554a1091b..0000000000 --- a/app/lib/primer/forms/primer_multi_input.d.ts +++ /dev/null @@ -1,10 +0,0 @@ -export declare class PrimerMultiInputElement extends HTMLElement { - fields: HTMLInputElement[]; - activateField(name: string): void; - private findField; -} -declare global { - interface Window { - PrimerMultiInputElement: typeof PrimerMultiInputElement; - } -} diff --git a/app/lib/primer/forms/primer_text_field.d.ts b/app/lib/primer/forms/primer_text_field.d.ts deleted file mode 100644 index 3e81f1e4c9..0000000000 --- a/app/lib/primer/forms/primer_text_field.d.ts +++ /dev/null @@ -1,28 +0,0 @@ -import '@github/auto-check-element'; -import type { AutoCheckErrorEvent, AutoCheckSuccessEvent } from '@github/auto-check-element'; -declare global { - interface HTMLElementEventMap { - 'auto-check-success': AutoCheckSuccessEvent; - 'auto-check-error': AutoCheckErrorEvent; - } -} -export declare class PrimerTextFieldElement extends HTMLElement { - #private; - inputElement: HTMLInputElement; - validationElement: HTMLElement; - validationMessageElement: HTMLElement; - validationSuccessIcon: HTMLElement; - validationErrorIcon: HTMLElement; - leadingVisual: HTMLElement; - leadingSpinner: HTMLElement; - connectedCallback(): void; - disconnectedCallback(): void; - clearContents(): void; - clearError(): void; - setValidationMessage(message: string): void; - toggleValidationStyling(isError: boolean): void; - setSuccess(message: string): void; - setError(message: string): void; - showLeadingSpinner(): void; - hideLeadingSpinner(): void; -} diff --git a/app/lib/primer/forms/primer_text_field.js b/app/lib/primer/forms/primer_text_field.js deleted file mode 100644 index 03d3e71a18..0000000000 --- a/app/lib/primer/forms/primer_text_field.js +++ /dev/null @@ -1,119 +0,0 @@ -/* eslint-disable custom-elements/expose-class-on-global */ -var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) { - var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d; - if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc); - else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r; - return c > 3 && r && Object.defineProperty(target, key, r), r; -}; -var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, state, kind, f) { - if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a getter"); - if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot read private member from an object whose class did not declare it"); - return kind === "m" ? f : kind === "a" ? f.call(receiver) : f ? f.value : state.get(receiver); -}; -var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, state, value, kind, f) { - if (kind === "m") throw new TypeError("Private method is not writable"); - if (kind === "a" && !f) throw new TypeError("Private accessor was defined without a setter"); - if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot write private member to an object whose class did not declare it"); - return (kind === "a" ? f.call(receiver, value) : f ? f.value = value : state.set(receiver, value)), value; -}; -var _PrimerTextFieldElement_abortController; -import '@github/auto-check-element'; -import { controller, target } from '@github/catalyst'; -let PrimerTextFieldElement = class PrimerTextFieldElement extends HTMLElement { - constructor() { - super(...arguments); - _PrimerTextFieldElement_abortController.set(this, void 0); - } - connectedCallback() { - __classPrivateFieldGet(this, _PrimerTextFieldElement_abortController, "f")?.abort(); - const { signal } = (__classPrivateFieldSet(this, _PrimerTextFieldElement_abortController, new AbortController(), "f")); - this.addEventListener('auto-check-success', async (event) => { - const message = await event.detail.response.text(); - if (message && message.length > 0) { - this.setSuccess(message); - } - else { - this.clearError(); - } - }, { signal }); - this.addEventListener('auto-check-error', async (event) => { - const errorMessage = await event.detail.response.text(); - this.setError(errorMessage); - }, { signal }); - } - disconnectedCallback() { - __classPrivateFieldGet(this, _PrimerTextFieldElement_abortController, "f")?.abort(); - } - clearContents() { - this.inputElement.value = ''; - this.inputElement.focus(); - this.inputElement.dispatchEvent(new Event('input', { bubbles: true, cancelable: false })); - } - clearError() { - this.inputElement.removeAttribute('invalid'); - this.validationElement.hidden = true; - this.validationMessageElement.replaceChildren(); - } - setValidationMessage(message) { - const template = document.createElement('template'); - // eslint-disable-next-line github/no-inner-html - template.innerHTML = message; - const fragment = document.importNode(template.content, true); - this.validationMessageElement.replaceChildren(fragment); - } - toggleValidationStyling(isError) { - if (isError) { - this.validationElement.classList.remove('FormControl-inlineValidation--success'); - } - else { - this.validationElement.classList.add('FormControl-inlineValidation--success'); - } - this.validationSuccessIcon.hidden = isError; - this.validationErrorIcon.hidden = !isError; - this.inputElement.setAttribute('invalid', isError ? 'true' : 'false'); - } - setSuccess(message) { - this.toggleValidationStyling(false); - this.setValidationMessage(message); - this.validationElement.hidden = false; - } - setError(message) { - this.toggleValidationStyling(true); - this.setValidationMessage(message); - this.validationElement.hidden = false; - } - showLeadingSpinner() { - this.leadingSpinner?.removeAttribute('hidden'); - this.leadingVisual?.setAttribute('hidden', ''); - } - hideLeadingSpinner() { - this.leadingSpinner?.setAttribute('hidden', ''); - this.leadingVisual?.removeAttribute('hidden'); - } -}; -_PrimerTextFieldElement_abortController = new WeakMap(); -__decorate([ - target -], PrimerTextFieldElement.prototype, "inputElement", void 0); -__decorate([ - target -], PrimerTextFieldElement.prototype, "validationElement", void 0); -__decorate([ - target -], PrimerTextFieldElement.prototype, "validationMessageElement", void 0); -__decorate([ - target -], PrimerTextFieldElement.prototype, "validationSuccessIcon", void 0); -__decorate([ - target -], PrimerTextFieldElement.prototype, "validationErrorIcon", void 0); -__decorate([ - target -], PrimerTextFieldElement.prototype, "leadingVisual", void 0); -__decorate([ - target -], PrimerTextFieldElement.prototype, "leadingSpinner", void 0); -PrimerTextFieldElement = __decorate([ - controller -], PrimerTextFieldElement); -export { PrimerTextFieldElement }; diff --git a/app/lib/primer/forms/toggle_switch_input.d.ts b/app/lib/primer/forms/toggle_switch_input.d.ts deleted file mode 100644 index 5dea4b0213..0000000000 --- a/app/lib/primer/forms/toggle_switch_input.d.ts +++ /dev/null @@ -1,5 +0,0 @@ -export declare class ToggleSwitchInputElement extends HTMLElement { - validationElement: HTMLElement; - validationMessageElement: HTMLElement; - connectedCallback(): void; -} diff --git a/package.json b/package.json index 9b83053c98..dd444c1d2b 100644 --- a/package.json +++ b/package.json @@ -27,9 +27,7 @@ "app/components/primer/**/*.css.json", "app/components/primer/**/*.d.ts", "app/lib/primer/forms/**/*.js", - "app/lib/primer/forms/**/*.d.ts", - "lib/primer/forms/**/*.js", - "lib/primer/forms/**/*.d.ts" + "app/lib/primer/forms/**/*.d.ts" ], "scripts": { "clean": "git clean -fdX -- app/", From b9cce66038a01ffa50a8f771f0b7d7312e567f88 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Mon, 14 Oct 2024 11:21:20 -0700 Subject: [PATCH 2/4] Allow form groups to accept system arguments (#3149) --- .changeset/hip-deers-rhyme.md | 5 +++++ app/lib/primer/forms/base_component.rb | 8 -------- app/lib/primer/forms/group.html.erb | 2 +- app/lib/primer/forms/group.rb | 5 +++++ test/lib/primer/forms/group_input_test.rb | 22 ++++++++++++++++++++++ test/lib/primer/forms_test.rb | 6 ------ 6 files changed, 33 insertions(+), 15 deletions(-) create mode 100644 .changeset/hip-deers-rhyme.md create mode 100644 test/lib/primer/forms/group_input_test.rb diff --git a/.changeset/hip-deers-rhyme.md b/.changeset/hip-deers-rhyme.md new file mode 100644 index 0000000000..58401478a6 --- /dev/null +++ b/.changeset/hip-deers-rhyme.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': minor +--- + +Allow form groups to accept system arguments diff --git a/app/lib/primer/forms/base_component.rb b/app/lib/primer/forms/base_component.rb index 428f4f72d4..0199afc8b1 100644 --- a/app/lib/primer/forms/base_component.rb +++ b/app/lib/primer/forms/base_component.rb @@ -61,14 +61,6 @@ def compile_and_render_template self.class.compile! unless self.class.instance_methods(false).include?(:render_template) render_template end - - def content_tag_if(condition, tag, **kwargs, &block) - if condition - content_tag(tag, **kwargs, &block) - else - capture(&block) - end - end end end end diff --git a/app/lib/primer/forms/group.html.erb b/app/lib/primer/forms/group.html.erb index d918111edc..58058165ad 100644 --- a/app/lib/primer/forms/group.html.erb +++ b/app/lib/primer/forms/group.html.erb @@ -1,4 +1,4 @@ -<%= content_tag_if(horizontal?, :div, class: "FormControl-horizontalGroup") do %> +<%= render(Primer::Box.new(**@system_arguments)) do %> <% @inputs.each do |input| %> <%= render(input.to_component) %> <% end %> diff --git a/app/lib/primer/forms/group.rb b/app/lib/primer/forms/group.rb index 889603470b..2b7239b7f8 100644 --- a/app/lib/primer/forms/group.rb +++ b/app/lib/primer/forms/group.rb @@ -17,6 +17,11 @@ def initialize(inputs:, builder:, form:, layout: DEFAULT_LAYOUT, **system_argume @form = form @layout = layout @system_arguments = system_arguments + + @system_arguments[:classes] = class_names( + @system_arguments.delete(:classes), + "FormControl-horizontalGroup" => horizontal? + ) end def horizontal? diff --git a/test/lib/primer/forms/group_input_test.rb b/test/lib/primer/forms/group_input_test.rb new file mode 100644 index 0000000000..623aee11f5 --- /dev/null +++ b/test/lib/primer/forms/group_input_test.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require "lib/test_helper" + +class Primer::Forms::GroupInputTest < Minitest::Test + include Primer::ComponentTestHelpers + + def test_group_accepts_system_arguments + render_in_view_context do + primer_form_with(url: "/foo") do |f| + render_inline_form(f) do |sys_args_form| + sys_args_form.group(layout: :horizontal, border: true, p: 1) do |group| + group.text_field(name: :first_name, label: "First name") + group.text_field(name: :last_name, label: "Last name") + end + end + end + end + + assert_selector ".FormControl-horizontalGroup.border.p-1" + end +end diff --git a/test/lib/primer/forms_test.rb b/test/lib/primer/forms_test.rb index d6f7ef111b..d024523dad 100644 --- a/test/lib/primer/forms_test.rb +++ b/test/lib/primer/forms_test.rb @@ -297,12 +297,6 @@ def test_text_field_custom_element_is_form_control assert_selector "primer-text-field.FormControl" end - def test_siblings_are_form_controls_when_including_a_multi_input - render_preview :multi_input_form - - assert_selector ".FormControl-radio-group-wrap + .FormControl" - end - def test_toggle_switch_button_labelled_by_label render_preview(:example_toggle_switch_form) From c8b9b4b7280746535a988e9e08c07ce8dd83223f Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Mon, 14 Oct 2024 11:23:23 -0700 Subject: [PATCH 3/4] Reduce ActionMenu test flakes (#3143) --- test/system/alpha/action_menu_test.rb | 153 ++++++++++++++------------ 1 file changed, 83 insertions(+), 70 deletions(-) diff --git a/test/system/alpha/action_menu_test.rb b/test/system/alpha/action_menu_test.rb index c97d991f1b..f9a61d6c6d 100644 --- a/test/system/alpha/action_menu_test.rb +++ b/test/system/alpha/action_menu_test.rb @@ -56,6 +56,28 @@ def click_on_fourth_item click_on_item(4) end + def retry_block(max_attempts: 3) + attempts = 1 + + begin + yield + rescue Minitest::Assertion => e + raise e if attempts >= max_attempts + attempts += 1 + retry + end + end + + def open_panel_via_keyboard + retry_block do + focus_on_invoker_button + + # open menu, "click" on first item + keyboard.type(:enter) + assert_selector "anchored-position:popover-open" # wait for menu to open + end + end + def close_dialog find("[data-close-dialog-id][aria-label=Close]").click end @@ -111,22 +133,22 @@ def test_action_js_onclick def test_action_js_keydown visit_preview(:with_actions) - focus_on_invoker_button + open_panel_via_keyboard accept_alert do # open menu, "click" on first item - keyboard.type(:enter, :enter) + keyboard.type(:enter) end end def test_action_js_keydown_space visit_preview(:with_actions) - focus_on_invoker_button + open_panel_via_keyboard accept_alert do # open menu, "click" on first item - keyboard.type(:enter, :space) + keyboard.type(:space) end end @@ -143,31 +165,29 @@ def test_action_js_disabled def test_action_js_disabled_keydown visit_preview(:with_actions, disable_items: true) - focus_on_invoker_button + open_panel_via_keyboard assert_no_alert do # open menu, "click" on first item - keyboard.type(:enter, :enter) + keyboard.type(:enter) end end def test_action_js_disabled_keydown_space visit_preview(:with_actions, disable_items: true) - focus_on_invoker_button + open_panel_via_keyboard assert_no_alert do # open menu, "click" on first item - keyboard.type(:enter, :space) + keyboard.type(:space) end end def test_action_keydown_on_icon_button visit_preview(:with_icon_button) - focus_on_invoker_button - - keyboard.type(:enter) + open_panel_via_keyboard assert_selector "anchored-position" end @@ -184,10 +204,10 @@ def test_action_anchor def test_action_anchor_keydown visit_preview(:with_actions) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, arrow down to second item, "click" second item - keyboard.type(:enter, :down, :enter) + # arrow down to second item, "click" second item + keyboard.type(:down, :enter) assert_selector ".action-menu-landing-page", text: "Hello world!" end @@ -195,10 +215,10 @@ def test_action_anchor_keydown def test_action_anchor_keydown_space visit_preview(:with_actions) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, arrow down to second item, "click" second item - keyboard.type(:enter, :down, :space) + # arrow down to second item, "click" second item + keyboard.type(:down, :space) assert_selector ".action-menu-landing-page", text: "Hello world!" end @@ -216,10 +236,10 @@ def test_action_anchor_disabled def test_action_anchor_disabled_keydown visit_preview(:with_actions, disable_items: true) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, arrow down to second item, "click" second item - keyboard.type(:enter, :down, :enter) + # arrow down to second item, "click" second item + keyboard.type(:down, :enter) # assert no navigation took place refute_selector ".action-menu-landing-page", text: "Hello world!" @@ -228,10 +248,10 @@ def test_action_anchor_disabled_keydown def test_action_anchor_disabled_keydown_space visit_preview(:with_actions, disable_items: true) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, arrow down to second item, "click" second item - keyboard.type(:enter, :down, :space) + # arrow down to second item, "click" second item + keyboard.type(:down, :space) # assert no navigation took place refute_selector ".action-menu-landing-page", text: "Hello world!" @@ -252,11 +272,11 @@ def test_action_clipboard_copy def test_action_clipboard_copy_keydown visit_preview(:with_actions) - focus_on_invoker_button + open_panel_via_keyboard clipboard_text = capture_clipboard do - # open menu, arrow down to third item, "click" third item - keyboard.type(:enter, :down, :down, :enter) + # arrow down to third item, "click" third item + keyboard.type(:down, :down, :enter) end assert_equal clipboard_text, "Text to copy" @@ -265,11 +285,11 @@ def test_action_clipboard_copy_keydown def test_action_clipboard_copy_keydown_space visit_preview(:with_actions) - focus_on_invoker_button + open_panel_via_keyboard clipboard_text = capture_clipboard do # open menu, arrow down to third item, "click" third item - keyboard.type(:enter, :down, :down, :space) + keyboard.type(:down, :down, :space) end assert_equal clipboard_text, "Text to copy" @@ -290,11 +310,11 @@ def test_action_clipboard_copy_disabled def test_action_clipboard_copy_disabled_keydown visit_preview(:with_actions, disable_items: true) - focus_on_invoker_button + open_panel_via_keyboard clipboard_text = capture_clipboard do - # open menu, arrow down to third item, "click" third item - keyboard.type(:enter, :down, :down, :enter) + # arrow down to third item, "click" third item + keyboard.type(:down, :down, :enter) end assert_nil clipboard_text @@ -303,11 +323,11 @@ def test_action_clipboard_copy_disabled_keydown def test_action_clipboard_copy_disabled_keydown_space visit_preview(:with_actions, disable_items: true) - focus_on_invoker_button + open_panel_via_keyboard clipboard_text = capture_clipboard do - # open menu, arrow down to third item, "click" third item - keyboard.type(:enter, :down, :down, :space) + # arrow down to third item, "click" third item + keyboard.type(:down, :down, :space) end assert_nil clipboard_text @@ -316,10 +336,7 @@ def test_action_clipboard_copy_disabled_keydown_space def test_first_item_is_focused_on_invoker_keydown visit_preview(:with_actions) - focus_on_invoker_button - - # open menu - keyboard.type(:enter) + open_panel_via_keyboard assert_equal page.evaluate_script("document.activeElement").text, "Alert" end @@ -360,10 +377,10 @@ def test_open_then_closing_dialog_restores_focus def test_opens_dialog_on_keydown visit_preview(:opens_dialog) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, arrow down to second item, "click" second item - keyboard.type(:enter, :down, :enter) + # arrow down to second item, "click" second item + keyboard.type(:down, :enter) assert_selector "dialog#my-dialog" end @@ -371,10 +388,10 @@ def test_opens_dialog_on_keydown def test_opens_dialog_on_keydown_space visit_preview(:opens_dialog) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, arrow down to second item, "click" second item - keyboard.type(:enter, :down, :space) + # arrow down to second item, "click" second item + keyboard.type(:down, :space) assert_selector "dialog#my-dialog" end @@ -463,10 +480,10 @@ def test_single_select_items_can_submit_forms def test_single_select_items_can_submit_forms_on_enter visit_preview(:single_select_form_items, route_format: :json) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, "click" first item - keyboard.type(:enter, :enter) + # "click" first item + keyboard.type(:enter) # for some reason the JSON response is wrapped in HTML, I have no idea why response = JSON.parse(find("pre").text) @@ -476,10 +493,10 @@ def test_single_select_items_can_submit_forms_on_enter def test_single_select_items_can_submit_forms_on_keydown_space visit_preview(:single_select_form_items, route_format: :json) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, "click" first item - keyboard.type(:enter, :space) + # "click" first item + keyboard.type(:space) # for some reason the JSON response is wrapped in HTML, I have no idea why response = JSON.parse(find("pre").text) @@ -512,9 +529,7 @@ def test_deferred_loading def test_deferred_loading_on_keydown visit_preview(:with_deferred_content) - focus_on_invoker_button - - keyboard.type(:enter) + open_panel_via_keyboard # wait for menu to load assert_selector "action-menu ul li", text: "Copy link" @@ -562,34 +577,32 @@ def test_single_select_item_checked def test_single_select_item_checked_via_keyboard_enter visit_preview(:single_select) - focus_on_invoker_button - - # open menu, "click" on first item - keyboard.type(:enter, :enter) + open_panel_via_keyboard + keyboard.type(:enter) # activating item closes menu, so checked item is hidden assert_selector "[aria-checked=true]", text: "Fast forward", visible: :hidden - focus_on_invoker_button + open_panel_via_keyboard + keyboard.type(:down, :enter) - keyboard.type(:enter, :down, :enter) assert_selector "[aria-checked=true]", text: "Recursive", visible: :hidden end def test_single_select_item_checked_via_keyboard_space visit_preview(:single_select) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, "click" on first item - keyboard.type(:enter, :space) + # "click" on first item + keyboard.type(:space) # activating item closes menu, so checked item is hidden assert_selector "[aria-checked=true]", text: "Fast forward", visible: :hidden - focus_on_invoker_button + open_panel_via_keyboard - keyboard.type(:enter, :down, :space) + keyboard.type(:down, :space) assert_selector "[aria-checked=true]", text: "Recursive", visible: :hidden end @@ -637,10 +650,10 @@ def test_multi_select_items_checked def test_multi_select_items_checked_via_keyboard_enter visit_preview(:multiple_select) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, select first item - keyboard.type(:enter, :enter) + # select first item + keyboard.type(:enter) assert_selector "[aria-checked=true]", text: "langermank" @@ -654,10 +667,10 @@ def test_multi_select_items_checked_via_keyboard_enter def test_multi_select_items_checked_via_keyboard_space visit_preview(:multiple_select) - focus_on_invoker_button + open_panel_via_keyboard - # open menu, select first item - keyboard.type(:enter, :space) + # select first item + keyboard.type(:space) assert_selector "[aria-checked=true]", text: "langermank" From 37e78c05e5e4ac1c8d482468b77a94b495aef6b3 Mon Sep 17 00:00:00 2001 From: Cameron Dutro Date: Mon, 14 Oct 2024 11:23:37 -0700 Subject: [PATCH 4/4] Allow setting custom values on submit buttons (#3141) --- .changeset/cuddly-games-prove.md | 5 +++ app/lib/primer/forms/button.rb | 7 +++-- app/lib/primer/forms/dsl/input.rb | 6 +++- .../primer/forms/submit_button_input_test.rb | 31 +++++++++++++++++++ 4 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 .changeset/cuddly-games-prove.md create mode 100644 test/lib/primer/forms/submit_button_input_test.rb diff --git a/.changeset/cuddly-games-prove.md b/.changeset/cuddly-games-prove.md new file mode 100644 index 0000000000..376c13dd6a --- /dev/null +++ b/.changeset/cuddly-games-prove.md @@ -0,0 +1,5 @@ +--- +'@primer/view-components': patch +--- + +Allow setting custom values on submit buttons. diff --git a/app/lib/primer/forms/button.rb b/app/lib/primer/forms/button.rb index b456420a3b..bc5a40ac3e 100644 --- a/app/lib/primer/forms/button.rb +++ b/app/lib/primer/forms/button.rb @@ -54,11 +54,14 @@ def input_arguments private def tag_attributes + attrs = { name: @input.name } + attrs[:value] = @input.value if @input.value + case @type when :submit - ButtonAttributeGenerator.submit_tag_attributes(@input.label, name: @input.name) + ButtonAttributeGenerator.submit_tag_attributes(@input.label, **attrs) else - ButtonAttributeGenerator.button_tag_attributes(@input.label, name: @input.name) + ButtonAttributeGenerator.button_tag_attributes(@input.label, **attrs) end end end diff --git a/app/lib/primer/forms/dsl/input.rb b/app/lib/primer/forms/dsl/input.rb index d1280e8aca..e022525a86 100644 --- a/app/lib/primer/forms/dsl/input.rb +++ b/app/lib/primer/forms/dsl/input.rb @@ -246,6 +246,10 @@ def id @input_arguments[:id] end + def value + @input_arguments[:value] + end + # :nocov: def name raise_for_abstract_method!(__method__) @@ -305,7 +309,7 @@ def input_data def caption_template_name return nil unless name - @caption_template_name ||= if respond_to?(:value) + @caption_template_name ||= if respond_to?(:value) && value.present? :"#{name}_#{value}" else name.to_sym diff --git a/test/lib/primer/forms/submit_button_input_test.rb b/test/lib/primer/forms/submit_button_input_test.rb new file mode 100644 index 0000000000..b8e925fafb --- /dev/null +++ b/test/lib/primer/forms/submit_button_input_test.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require "lib/test_helper" + +class Primer::Forms::SubmitButtonInputTest < Minitest::Test + include Primer::ComponentTestHelpers + + def test_uses_name_as_value_by_default + render_in_view_context do + primer_form_with(url: "/foo") do |f| + render_inline_form(f) do |submit_button_form| + submit_button_form.submit(name: :foo, label: "Foo") + end + end + end + + assert_selector "button[type=submit][value=Foo]" + end + + def test_allows_overriding_value + render_in_view_context do + primer_form_with(url: "/foo") do |f| + render_inline_form(f) do |submit_button_form| + submit_button_form.submit(name: :foo, label: "Foo", value: "bar") + end + end + end + + assert_selector "button[type=submit][value=bar]" + end +end