Skip to content

Commit

Permalink
Improve interop with Rails FormBuilder/form_with
Browse files Browse the repository at this point in the history
* Teaches `form_arguments` `:builder` and `:name` options. `:builder`
  should be an instance of `ActionView::Helpers::FormBuilder`, which is
  created by the standard Rails `#form_with` and `#form_for` helpers.
* Uses Rails Form Helpers under-the-hood to render form tags, ensuring
  automatic generation of CSRF token, `_method` hidden inputs.
  • Loading branch information
myabc committed Dec 20, 2024
1 parent 59fef6e commit f12553a
Show file tree
Hide file tree
Showing 11 changed files with 139 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
</scrollable-region>
<%= render(Primer::Alpha::Dialog::Footer.new(show_divider: false)) do %>
<%= render(Primer::Beta::Button.new(data: { "close-dialog-id": dialog_id })) { I18n.t("button_close") } %>
<%= render(Primer::Beta::Button.new(type: (show_form? ? :submit : :button), scheme: :danger, data: { "submit-dialog-id": dialog_id })) { I18n.t("button_delete_permanently") } %>
<%= render(Primer::Beta::Button.new(type: (@form_wrapper.shows_form? ? :submit : :button), scheme: :danger, data: { "submit-dialog-id": dialog_id })) { I18n.t("button_delete_permanently") } %>
<% end %>
<% end %>
</danger-confirmation-dialog-form-helper>
Expand Down
18 changes: 8 additions & 10 deletions app/components/primer/open_project/danger_confirmation_dialog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class DangerConfirmationDialog < Primer::Component

check_box_id = "#{dialog_id}-check_box"

Primer::OpenProject::DangerConfirmationDialog::ConfirmationCheckBox.new(check_box_id: check_box_id, **system_arguments)
Primer::OpenProject::DangerConfirmationDialog::ConfirmationCheckBox.new(check_box_id: check_box_id, check_box_name: @check_box_name, **system_arguments)
}

# Optional additional_details such as grid displaying a list of items to be deleted
Expand All @@ -57,18 +57,20 @@ class DangerConfirmationDialog < Primer::Component
Primer::BaseComponent.new(**system_arguments)
}

# @param form_arguments [Hash] Allows the dialog to submit a form on click.
# @param form_arguments [Hash] Allows the dialog to submit a form. Pass EITHER the `builder:` option to this hash
# to reuse an existing Rails form, or `action:` if you prefer the component to render the form tag itself.
# `builder:` should be an instance of `ActionView::Helpers::FormBuilder`, which is created by the standard Rails
# `#form_with` and `#form_for` helpers. The `name:` option is the desired name of the field that will be included
# in the params sent to the server on form submission.
# @param id [String] The id of the dialog.
# @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
def initialize(
form_arguments: {},
id: self.class.generate_id,
**system_arguments
)
form_arguments = deny_tag_argument(**form_arguments)
@show_form = form_arguments.any?
@form_wrapper = Primer::ConditionalWrapper.new(condition: @show_form, **form_arguments.merge(tag: "form"))

@check_box_name = form_arguments.delete(:name) || "confirm_dangerous_action"
@form_wrapper = FormWrapper.new(**form_arguments)
@dialog_id = id.to_s

@system_arguments = system_arguments
Expand All @@ -85,10 +87,6 @@ def initialize(
:show_button?, :show_button, :with_show_button, :with_show_button_content,
to: :@dialog

def show_form?
@show_form
end

def render?
raise ArgumentError, "DangerConfirmationDialog requires a confirmation_message" unless confirmation_message?
raise ArgumentError, "DangerConfirmationDialog requires a confirmation_check_box" unless confirmation_check_box?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ class DangerConfirmationDialog
class ConfirmationCheckBox < Primer::Component

# @param check_box_id [String] The id of the check_box input.
# @param check_box_name [String] The name of the check_box input.
# @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
def initialize(check_box_id: self.class.generate_id, **system_arguments)
def initialize(check_box_id: self.class.generate_id, check_box_name:, **system_arguments)
@system_arguments = deny_tag_argument(**system_arguments)
@system_arguments[:tag] = :div
@system_arguments[:classes] = class_names(
Expand All @@ -19,7 +20,7 @@ def initialize(check_box_id: self.class.generate_id, **system_arguments)

@check_box_arguments = {}
@check_box_arguments[:id] = check_box_id
@check_box_arguments[:name] = "confirm_dangerous_action"
@check_box_arguments[:name] = check_box_name
@check_box_arguments[:data] = {
target: "danger-confirmation-dialog-form-helper.checkbox",
action: "change:danger-confirmation-dialog-form-helper#toggle"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<% if renders_form? %>
<%= form_with(url: @action, method: @http_method, **@form_arguments) do %>
<%= content %>
<% end %>
<% else %>
<%= content %>
<% end %>
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true

module Primer
module OpenProject
class DangerConfirmationDialog
# Utility component for wrapping DangerConfirmationDialog in a form
class FormWrapper < Primer::Component
def initialize(builder: nil, action: nil, **form_arguments)
raise ArgumentError, "Pass in either a :builder or :action argument, not both." if builder && action

@builder = builder
@action = action
@form_arguments = deny_tag_argument(**form_arguments)
end

def renders_form?
!@builder && @action
end

def shows_form?
@builder || @action
end
end
end
end
end
21 changes: 6 additions & 15 deletions previews/primer/open_project/danger_confirmation_dialog_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,14 @@ def playground(
check_box_text: check_box_text })
end

# @label With form using FormBuilder
def with_form_builder_form(route_format: :html)
render_with_template(locals: { route_format: route_format })
end

# @label With form
def with_form(route_format: :html)
render(Primer::OpenProject::DangerConfirmationDialog.new(
title: "Delete dialog",
form_arguments: {
method: :post,
action: generic_form_submission_path(format: route_format),
novalidate: true
}
)) do |dialog|
dialog.with_show_button { "Click me" }
dialog.with_confirmation_message do |message|
message.with_heading(tag: :h2).with_content("Permanently delete?")
message.with_description_content("This action is not reversible. Please proceed with caution.")
end
dialog.with_confirmation_check_box_content("I understand that this deletion cannot be reversed")
end
render_with_template(locals: { route_format: route_format })
end

# @label With additional details
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<%= render(Primer::OpenProject::DangerConfirmationDialog.new(
title: "Delete dialog",
form_arguments: { action: generic_form_submission_path(format: route_format) }
)) do |dialog| %>
<% dialog.with_show_button { "Click me" } %>
<% dialog.with_confirmation_message do |message|
message.with_heading(tag: :h2).with_content("Permanently delete?")
message.with_description_content("This action is not reversible. Please proceed with caution.")
end %>
<% dialog.with_confirmation_check_box_content("I understand that this deletion cannot be reversed") %>
<% end %>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<%= form_with(url: generic_form_submission_path(format: route_format)) do |f| %>
<%= render(Primer::OpenProject::DangerConfirmationDialog.new(
title: "Delete dialog",
form_arguments: { builder: f, name: "confirm_very_dangerous_action" }
)) do |dialog| %>
<% dialog.with_show_button { "Click me" } %>
<% dialog.with_confirmation_message do |message|
message.with_heading(tag: :h2).with_content("Permanently delete?")
message.with_description_content("This action is not reversible. Please proceed with caution.")
end %>
<% dialog.with_confirmation_check_box_content("I understand that this deletion cannot be reversed") %>
<% end %>
<% end %>
3 changes: 2 additions & 1 deletion test/components/component_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ def test_registered_components
"Primer::OpenProject::PageHeader::Menu",
"Primer::OpenProject::PageHeader::Dialog",
"Primer::OpenProject::PageHeader::Title",
"Primer::OpenProject::SidePanel::Section"
"Primer::OpenProject::SidePanel::Section",
"Primer::OpenProject::DangerConfirmationDialog::FormWrapper"
]

primer_component_files_count = Dir["app/components/**/*.rb"].count { |p| p.exclude?("/experimental/") }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,38 @@ def test_renders_button_type_buttons_by_default
end
end

def test_renders_form_with_form_arguments
render_inline(Primer::OpenProject::DangerConfirmationDialog.new(
form_arguments: { action: "/my-action", method: :delete }
)) do |dialog|
dialog.with_confirmation_message do |message|
message.with_heading(tag: :h2) { "Danger" }
def test_raises_argument_error_with_invalid_form_arguments
error = assert_raises do
render_in_view_context do
form_with(url: "/my-action", method: :delete) do |f|
render(Primer::OpenProject::DangerConfirmationDialog.new(form_arguments: { builder: f, action: "/my-action" })) do |dialog|
dialog.with_confirmation_message do |message|
message.with_heading(tag: :h2) { "Danger" }
end
dialog.with_confirmation_check_box { "I confirm this deletion" }
end
end
end
dialog.with_confirmation_check_box { "I confirm this deletion" }
end

assert_equal "Pass in either a :builder or :action argument, not both.", error.message
end

def test_renders_form_with_form_arguments
render_inline(Primer::OpenProject::DangerConfirmationDialog.new(
form_arguments: { action: "/my-action", method: :delete, name: "custom_check" }
)) do |dialog|
dialog.with_confirmation_message do |message|
message.with_heading(tag: :h2) { "Danger" }
end
dialog.with_confirmation_check_box { "I confirm this deletion" }
end

assert_selector("dialog.DangerConfirmationDialog") do
assert_selector("form[action='/my-action'][method='delete']")
assert_selector("form[action='/my-action']") do
assert_selector("input[type='hidden'][name='_method'][value='delete']", visible: false)
assert_selector("input[type='checkbox'][name='custom_check']")
end
end
end

Expand All @@ -101,6 +121,24 @@ def test_renders_submit_type_button_with_form_arguments
end
end

def test_renders_submit_type_button_with_form_builder_form_arguments
render_in_view_context do
form_with(url: "/my-action", method: :delete) do |f|
render(Primer::OpenProject::DangerConfirmationDialog.new(form_arguments: { builder: f })) do |dialog|
dialog.with_confirmation_message do |message|
message.with_heading(tag: :h2) { "Danger" }
end
dialog.with_confirmation_check_box { "I confirm this deletion" }
end
end
end

assert_selector("dialog.DangerConfirmationDialog") do
assert_selector("button[type='submit']")
assert_selector("button[type='button']", count: 2)
end
end

def test_renders_provided_id
render_inline(Primer::OpenProject::DangerConfirmationDialog.new(id: "danger-dialog")) do |dialog|
dialog.with_confirmation_message do |message|
Expand Down
17 changes: 16 additions & 1 deletion test/system/open_project/danger_confirmation_dialog_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,22 @@ def test_submit_button_submits_form

# for some reason the JSON response is wrapped in HTML, I have no idea why
response = JSON.parse(find("pre").text)
assert_equal "fast_forward", response["value"]
assert_equal "1", response.dig("form_params", "confirm_dangerous_action")
end
end

def test_submit_button_submits_form_builder_form
visit_preview(:with_form_builder_form, route_format: :json)

click_button("Click me")

assert_selector(".DangerConfirmationDialog") do
check("I understand that this deletion cannot be reversed")
find("button[type='submit']").click

# for some reason the JSON response is wrapped in HTML, I have no idea why
response = JSON.parse(find("pre").text)
assert_equal "1", response.dig("form_params", "confirm_very_dangerous_action")
end
end
end

0 comments on commit f12553a

Please sign in to comment.