Skip to content

Commit

Permalink
Unsafe Actions: Replace link_to calls with button_to
Browse files Browse the repository at this point in the history
The `<a>` element is suitable for [Safe HTTP Methods][] (like `GET`) to
drive page navigations. Unsafe HTTP Methods (like `POST`, `PUT`, and
`DELETE`) are better initiated by `<form>` submissions.

This commit replaces generated calls to `link_to` with calls to
`button_to`.

The rest of the styling changes aim to preserve design decisions made
about preserving the appearance of elements that were once presented as
`<a>` elements that are now presented as `<input type="submit">`
elements nested within `<form>` elements.

While the design changes preserved backwards compatibility, it's worth
re-considering the choice to present them as "navigation" links instead
of "action" buttons.

[Safe HTTP Methods]: https://developer.mozilla.org/en-US/docs/Glossary/Safe/HTTP
  • Loading branch information
seanpdoyle committed Feb 14, 2024
1 parent bba744f commit df4c1df
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 39 deletions.
55 changes: 34 additions & 21 deletions app/assets/builds/administrate/application.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion app/assets/builds/administrate/application.css.map

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions app/assets/stylesheets/administrate/base/_layout.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ figure {
margin: 0;
}

form.button_to {
display: contents;
}

img,
picture {
margin: 0;
Expand Down
3 changes: 2 additions & 1 deletion app/assets/stylesheets/administrate/base/_typography.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ p {
margin: 0 0 $small-spacing;
}

a {
a,
.link {
color: $action-color;
text-decoration-skip-ink: auto;
transition: color $base-duration $base-timing;
Expand Down
23 changes: 15 additions & 8 deletions app/assets/stylesheets/administrate/components/_buttons.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,34 @@ button,
input,
.button {
appearance: none;
background-color: $action-color;
border: 0;
border-radius: $base-border-radius;
color: $white;
cursor: pointer;
display: inline-block;
font-family: $base-font-family;
font-size: $base-font-size;
-webkit-font-smoothing: antialiased;
font-weight: $bold-font-weight;
line-height: 1;
padding: $small-spacing $base-spacing;
text-decoration: none;
transition: background-color $base-duration $base-timing;
user-select: none;
vertical-align: middle;
white-space: nowrap;

&:disabled {
cursor: not-allowed;
opacity: 0.5;
}
}

button:not(.link),
input:not(.link),
.button {
background-color: $action-color;
border-radius: $base-border-radius;
color: $white;
font-weight: $bold-font-weight;
padding: $small-spacing $base-spacing;

&:hover {
background-color: mix($black, $action-color, 20%);
color: $white;
Expand All @@ -31,9 +41,6 @@ input,
}

&:disabled {
cursor: not-allowed;
opacity: 0.5;

&:hover {
background-color: $action-color;
}
Expand Down
3 changes: 2 additions & 1 deletion app/assets/stylesheets/administrate/reset/_normalize.scss
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ pre {
* 2. Remove gaps in links underline in iOS 8+ and Safari 8+.
*/

a {
a,
.link {
background-color: transparent; /* 1 */
-webkit-text-decoration-skip: objects; /* 2 */
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
<% end %>

<% if existing_action?(collection_presenter.resource_name, :destroy) %>
<td><%= link_to(
<td><%= button_to(
t("administrate.actions.destroy"),
[namespace, resource],
class: "text-color-red",
data: { turbo_method: :delete, turbo_confirm: t("administrate.actions.confirm") }
class: "link text-color-red",
method: :delete,
data: { turbo_confirm: t("administrate.actions.confirm") }
) if accessible_action?(resource, :destroy) %></td>
<% end %>
5 changes: 3 additions & 2 deletions app/views/administrate/application/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ as well as a link to its edit page.
class: "button",
) if accessible_action?(page.resource, :edit) %>

<%= link_to(
<%= button_to(
t("administrate.actions.destroy"),
[namespace, page.resource],
class: "button button--danger",
data: { turbo_method: :delete, turbo_confirm: t("administrate.actions.confirm") }
method: :delete,
data: { turbo_confirm: t("administrate.actions.confirm") }
) if accessible_action?(page.resource, :destroy) %>
</div>
</header>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
<% end %>

<% if existing_action?(collection_presenter.resource_name, :destroy) %>
<td><%= link_to(
<td><%= button_to(
t("administrate.actions.destroy"),
[namespace, resource],
class: "text-color-red",
class: "link text-color-red",
method: :delete,
data: { turbo_confirm: t("administrate.actions.confirm") }
) if authorized_action?(resource, :destroy) %></td>
Expand Down

0 comments on commit df4c1df

Please sign in to comment.