Doubtful usage of `<fieldset>` in dashboard show page
Uaitt opened this issue · 5 comments
Description
With Administrate v0.20.0, now it's possible to visually group together fields of the same model (introduced in this PR).
While I like the feature, I think that there is a small conceptual problem.
In both the app/views/administrate/application/_form.html.erb and app/view/administrate/application/show.html.erb a <fieldset> element was introduced (indeed, in order to group fields together).
However, most documentation (MDN, W3School, W3 says that this element "groups together element in a form"). With that being said, I really don't think this element should be used in the show view (while I totally agree with using it on the _form partial).
I am writing this also because I have noticed a strange problem on one of my current projects after this update to the <fieldset> in the show view, that occurs when one of the fields contain an horizontal scrollbar.
Show page before the change:
As you see, everything is properly aligned.
Show page after the change:
Layout is completely broken (I had to zoom out a lot in order to see the actual fields). The layout is fixed if the <fieldset> element is replaced with a generic container, like a <div> or <section>.
I would thus propose to replace the <fieldset> element in app/view/administrate/application/show.html.erb with a simple container (either <div> or <section>) and style it similar to a <fieldset>.
Versions
- Administrate v0.21.0
- Rails 7.1.3
- Ruby 3.2.2
Oh! Yes, I totally agree.
I do think a <section> is the right thing semantically. Would be able to contribute a PR to fix this?
@nickcharlton Yep, I will work on it 😃.
Hey 👋
Any update about this issue?
From my standpoint, the dl tag should have only dt and dd children - also w3 describe the dt tag as:
Content model: Zero or more groups each consisting of one or more dt elements followed by one or more dd elements
I'm asking because it breaks the styles of a theme plugin of mine 😅 and I have to introduce weird styles to handle a fieldset child...
On #2504, we've got some questions around the semantically correct way to handle this on the show page.
Hi @blocknotes and @nickcharlton, thanks for the follow up on this issue 😃.
Unfortunately, it's been a while since I have last used Administrate and because of that I don't think I can consider myself comfortable enough to continue with the original PR that tried to fix this.
Perhaps @blocknotes you may want to open a PR for the fix yourself and I'll close mine?