Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace <fieldset> with <section> on show pages #2504

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Replace <fieldset> with <section> on show pages #2504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions app/assets/stylesheets/administrate/components/_attributes.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,14 @@

.attribute--nested {
border: $base-border;
display: table;
padding: $small-spacing;
width: 100%;
}

.attribute--nested--legend {
background: $white;
display: inline;
position: relative;
top: -1.5em;
}
13 changes: 13 additions & 0 deletions app/assets/stylesheets/administrate/components/_field-unit.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,16 @@
content: " *";
}
}

section .field-unit--nested {
display: table;
margin-bottom: 1rem;
width: 90%;
}

.field-unit--legend {
background: $white;
display: inline;
position: relative;
top: -1.5rem;
}
10 changes: 7 additions & 3 deletions app/views/administrate/application/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,13 @@ as well as a link to its edit page.
<section class="main-content__body">
<dl>
<% page.attributes.each do |title, attributes| %>
<fieldset class="<%= "field-unit--nested" if title.present? %>">
<section class="<%= "field-unit--nested" if title.present? %>">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semantically, <fieldset> elements typically wrap interactive form controls. Since the show templates present information without the opportunity to edit that information, this change improves the semantics of what these elements are communicating.

Having acknowledged that, rendering multiple <h1> elements poses some accessibility risks, and nesting a <nav> element (a role that comes with its own set of semantics and requirements) inside an <h1> poses additional risks.

Are there other structural changes we could make to achieve a similar outcome without the risks?

Copy link
Contributor Author

@Uaitt Uaitt Feb 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seanpdoyle what element (or elements) would you recommend to replace my <h1> and <nav> combination? Do you have something in particular in mind? 🤔

<% if title.present? %>
<legend><%= t "helpers.label.#{page.resource_name}.#{title}", default: title %></legend>
<h1>
<nav class="<%= "field-unit--legend" %>">
<%= t "helpers.label.#{page.resource_name}.#{title}", default: title %>
</nav>
</h1>
<% end %>

<% attributes.each do |attribute| %>
Expand All @@ -59,7 +63,7 @@ as well as a link to its edit page.
<dd class="attribute-data attribute-data--<%=attribute.html_class%>"
><%= render_field attribute, page: page %></dd>
<% end %>
</fieldset>
</section>
<% end %>
</dl>
</section>
20 changes: 13 additions & 7 deletions app/views/fields/has_one/_show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,24 @@ All show page attributes of has_one relationship would be rendered
%>

<% if field.linkable? %>
<fieldset class="attribute--nested">
<legend>
<section class="attribute--nested">
<h1>
<nav class="attribute--nested--legend">
<%= link_to_if(
accessible_action?(field.data, :show),
field.display_associated_resource,
[namespace, field.data],
) %>
</legend>
</nav>
</h1>
<% field.nested_show.attributes.each do |title, attributes| -%>
<fieldset class="<%= "field-unit--nested" if title.present? %>">
<section class="<%= "field-unit--nested" if title.present? %>">
<% if title.present? %>
<legend><%= t "helpers.label.#{namespace}.#{title}", default: title %></legend>
<h1>
<nav class="<%= "field-unit--legend" %>">
<%= t "helpers.label.#{namespace}.#{title}", default: title %>
</nav>
</h1>
<% end %>

<% attributes.each do |attribute| %>
Expand All @@ -41,7 +47,7 @@ All show page attributes of has_one relationship would be rendered
<%= render_field attribute, { page: page } %>
</dd>
<% end %>
</fieldset>
</section>
<% end -%>
</fieldset>
</section>
<% end %>
Loading