Use a button for non-GET table actions

Links acting like buttons have a few disadvantages.

First, screen readers will announce them as "links". Screen reader users
usually associate links with "things that get you somewhere" and buttons
with "things that perform an action". So when something like "Delete,
link" is announced, they'll probably think this is a link which will
take them to another page where they can delete a record.

Furthermore, the URL of the link for the "destroy" action might be the
same as the URL for the "show" action (only one is accessed with a
DELETE request and the other one with a GET request). That means screen
readers could announce the link like "Delete, visited link", which is
very confusing.

They also won't work when opening links in a new tab, since opening
links in a new tab always results in a GET request to the URL the link
points to.

Finally, submit buttons work without JavaScript enabled, so they'll work
even if the JavaScript in the page hasn't loaded (for whatever reason).

For all these reasons (and probably many more), using a button to send
forms is IMHO superior to using links.

There's one disadvantage, though. Using `button_to` we create a <form>
tag, which means we'll generate invalid HTML if the table is inside
another form. If we run into this issue, we need to use `button_tag`
with a `form` attribute and then generate a form somewhere else inside
the HTML (with `content_for`).

Note we're using `button_to` with a block so it generates a <button>
tag. Using it in a different way the text would result in an <input />
tag, and input elements can't have pseudocontent added via CSS.

The following code could be a starting point to use the `button_tag`
with a `form` attribute. One advantage of this approach is screen
readers wouldn't announce "leaving form" while navigating through these
buttons. However, it doesn't work in Internet Explorer.

```
ERB:

<% content_for(:hidden_content, form_tag(path, form_options) {}) %>
<%= button_tag text, button_options %>

Ruby:

def form_id
  path.gsub("/", "_")
end

def form_options
  { id: form_id, method: options[:method] }
end

def button_options
  html_options.except(:method).merge(form: form_id)
end

Layout:

<%= content_for :hidden_content %> # Right before the `</body>`
```
This commit is contained in:
Javi Martín
2021-08-18 13:30:02 +02:00
parent 6510cb9615
commit 5311daadfe
50 changed files with 193 additions and 166 deletions

View File

@@ -61,11 +61,11 @@ describe "Admin booths assignments", :admin do
expect(page).to have_content(booth.name)
expect(page).to have_content "Unassigned"
click_link "Assign booth"
click_button "Assign booth"
expect(page).not_to have_content "Unassigned"
expect(page).to have_content "Assigned"
expect(page).to have_link "Unassign booth"
expect(page).to have_button "Unassign booth"
end
visit admin_poll_path(poll)
@@ -100,11 +100,11 @@ describe "Admin booths assignments", :admin do
expect(page).to have_content(booth.name)
expect(page).to have_content "Assigned"
click_link "Unassign booth"
click_button "Unassign booth"
expect(page).to have_content "Unassigned"
expect(page).not_to have_content "Assigned"
expect(page).to have_link "Assign booth"
expect(page).to have_button "Assign booth"
end
visit admin_poll_path(poll)
@@ -127,11 +127,11 @@ describe "Admin booths assignments", :admin do
expect(page).to have_content(booth.name)
expect(page).to have_content "Assigned"
accept_confirm { click_link "Unassign booth" }
accept_confirm { click_button "Unassign booth" }
expect(page).to have_content "Unassigned"
expect(page).not_to have_content "Assigned"
expect(page).to have_link "Assign booth"
expect(page).to have_button "Assign booth"
end
end
@@ -143,8 +143,7 @@ describe "Admin booths assignments", :admin do
within("#poll_booth_#{booth.id}") do
expect(page).to have_content(booth.name)
expect(page).to have_content "Assigned"
expect(page).not_to have_link "Unassign booth"
expect(page).not_to have_button "Unassign booth"
end
end
end

View File

@@ -19,14 +19,16 @@ describe "Admin poll officers", :admin do
click_button "Search"
expect(page).to have_content user.name
click_link "Add"
click_button "Add"
within("#officers") do
expect(page).to have_content user.name
end
end
scenario "Delete" do
accept_confirm { click_link "Delete position" }
accept_confirm { click_button "Delete position" }
expect(page).not_to have_css "#officers"
end

View File

@@ -119,7 +119,7 @@ describe "Admin polls", :admin do
visit admin_polls_path
within("#poll_#{poll.id}") do
accept_confirm { click_link "Delete" }
accept_confirm { click_button "Delete" }
end
expect(page).to have_content("Poll deleted successfully")
@@ -133,7 +133,7 @@ describe "Admin polls", :admin do
visit admin_polls_path
within(".poll", text: "Do you support CONSUL?") do
accept_confirm { click_link "Delete" }
accept_confirm { click_button "Delete" }
end
expect(page).to have_content("Poll deleted successfully")
@@ -150,7 +150,7 @@ describe "Admin polls", :admin do
visit admin_polls_path
within(".poll", text: "Do you support CONSUL?") do
accept_confirm { click_link "Delete" }
accept_confirm { click_button "Delete" }
end
expect(page).to have_content "Poll deleted successfully"
@@ -164,7 +164,7 @@ describe "Admin polls", :admin do
visit admin_polls_path
within("#poll_#{poll.id}") do
accept_confirm { click_link "Delete" }
accept_confirm { click_button "Delete" }
end
expect(page).to have_content("You cannot delete a poll that has votes")

View File

@@ -28,7 +28,7 @@ describe "Documents", :admin do
visit admin_answer_documents_path(answer)
expect(page).to have_content(document.title)
accept_confirm { click_link "Delete" }
accept_confirm { click_button "Delete" }
expect(page).not_to have_content(document.title)
end

View File

@@ -17,7 +17,7 @@ describe "Admin poll questions", :admin do
expect(page).to have_content(question1.title)
expect(page).to have_link "Edit answers"
expect(page).to have_link "Edit"
expect(page).to have_link "Delete"
expect(page).to have_button "Delete"
end
visit admin_poll_path(poll2)
@@ -27,7 +27,7 @@ describe "Admin poll questions", :admin do
expect(page).to have_content question2.title
expect(page).to have_link "Edit answers"
expect(page).to have_link "Edit"
expect(page).to have_link "Delete"
expect(page).to have_button "Delete"
end
visit admin_poll_path(poll3)
@@ -38,7 +38,7 @@ describe "Admin poll questions", :admin do
expect(page).to have_link "(See proposal)", href: proposal_path(question3.proposal)
expect(page).to have_link "Edit answers"
expect(page).to have_link "Edit"
expect(page).to have_link "Delete"
expect(page).to have_button "Delete"
end
end
@@ -142,7 +142,7 @@ describe "Admin poll questions", :admin do
visit admin_poll_path(poll)
within("#poll_question_#{question1.id}") do
accept_confirm { click_link "Delete" }
accept_confirm { click_button "Delete" }
end
expect(page).not_to have_content(question1.title)

View File

@@ -174,7 +174,7 @@ describe "Admin shifts", :admin do
expect(page).to have_css(".shift", count: 1)
within("#shift_#{shift.id}") do
accept_confirm { click_link "Remove" }
accept_confirm { click_button "Remove" }
end
expect(page).to have_content "Shift removed"
@@ -198,7 +198,7 @@ describe "Admin shifts", :admin do
expect(page).to have_css(".shift", count: 1)
within("#shift_#{shift.id}") do
accept_confirm { click_link "Remove" }
accept_confirm { click_button "Remove" }
end
expect(page).not_to have_content "Shift removed"
@@ -225,7 +225,7 @@ describe "Admin shifts", :admin do
expect(page).to have_css(".shift", count: 1)
within("#shift_#{shift.id}") do
accept_confirm { click_link "Remove" }
accept_confirm { click_button "Remove" }
end
expect(page).not_to have_content "Shift removed"