#1168246: Freedom For Fieldsets! Long Live The DETAILS. changed fieldsets to details.

The HTML5 spec says that details are always collapsible.

Native user agent implementations are following the spec. This means that the #collapsible property is obsolete.

Follow-up issue:
#1892182: #type details: Rename #collapsed to #open

Related:
#1839158: Replace collapse.js with a proper polyfill for <details>
#1829202: Make #type 'item' work outside of a form context to render a compound label + content
#1886728: Switch from details to fieldset when collapsing isn't needed

CommentFileSizeAuthor
#22 1852104-22.patch41.15 KBswentel
#18 1852104-18.patch41.15 KBstar-szr
#18 interdiff.txt1.01 KBstar-szr
#16 core-type-details-1852104-16.patch41.06 KBnod_
#13 core-type-details-1852104-13.patch40.95 KBnod_
#11 type.details.11.patch41.36 KBsun
#11 interdiff.txt1.15 KBsun
#2 type.details.1.patch43.68 KBsun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Grayside’s picture

Grayside CreditAttribution: Grayside commented
Title: Remove #collapsible property from #type details, and rename #collapsible to #open » Remove #collapsible property from #type details, and rename #collapsed to #open
sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Title: Remove #collapsible property from #type details, and rename #collapsed to #open » Remove #collapsible property from #type details, and rename #collapsible to #open
Assigned: Unassigned » sun
Status: Active » Needs review
FileSize
type.details.1.patch43.68 KB

This is just the first part only... now I remember why I left that out of the original issue/patch... ;)

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Title: Remove #collapsible property from #type details, and rename #collapsible to #open » Remove #collapsible property from #type details, and rename #collapsed to #open

x-post.

Status: Needs review » Needs work

The last submitted patch, type.details.1.patch, failed testing.

tkoleary’s picture

tkoleary CreditAttribution: tkoleary commented

@sun Could we also please only make the first summary/details on a page #open TRUE, and the successive ones #open FALSE? Opening them all kind of defeats the purpose right?

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

@tkoleary: The open attribute of details is controlled by each element on its own, by design. Whether it makes sense to open details is implementation and use-case-specific. Currently, we still have a lot of #type details elements that shouldn't be details in the first place though. They should be fieldsets or not grouped at all. Perhaps you're referring to those.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

#1886728: Switch from details to fieldset when collapsing isn't needed is an example of something that should be a fieldset again

YesCT’s picture

YesCT CreditAttribution: YesCT commented

Yes, #6 makes sense.

tkoleary’s picture

tkoleary CreditAttribution: tkoleary commented

@sun I see. I thought that summary and details was an effort to make us more semantic and replace fieldsets. What are the criteria for when summary and details are appropriate vs fieldsets?

tkoleary’s picture

tkoleary CreditAttribution: tkoleary commented

@Sun Just looked at #1886728 which spurred me to think this through more deeply. Seems to me that the purpose of summary and details is to provide a semantic underpinning for things that expand to reveal more information. If that is the case then logic dictates we should be S&D everywhere in place of collapsible fieldsets (collapsible being the key word; styled divs that do not collapse would be fine).

Then the only remaining question is "what are the criteria for determining when detatils open='true' vs.'false' "? so we can implement that in core and add to HIG.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Title: Remove #collapsible property from #type details, and rename #collapsed to #open » #type details: Remove #collapsible property
Status: Needs work » Needs review
FileSize
interdiff.txt1.15 KB
type.details.11.patch41.36 KB

Let's handle these two tasks in two separate steps: #1892182: #type details: Rename #collapsed to #open

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Needs review » Reviewed & tested by the community

Great!

swenteldoos-2:drupal swentel$ git apply type.details.11.patch 
swenteldoos-2:drupal swentel$ rm type.details.11.patch 
swenteldoos-2:drupal swentel$ grep -R '#collapsible' *
swenteldoos-2:drupal swentel$

Should be fine when it comes back.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
FileSize
core-type-details-1852104-13.patch40.95 KB

need reroll because of #1886728: Switch from details to fieldset when collapsing isn't needed Leaving RTBC, there is one #collapsible remove less from simpletest.pages.inc

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Issue tags: -API clean-up

#13: core-type-details-1852104-13.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work
Issue tags: +API clean-up

The last submitted patch, core-type-details-1852104-13.patch, failed testing.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs work » Needs review
FileSize
core-type-details-1852104-16.patch41.06 KB

reroll

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Reviewed & tested by the community
star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
FileSize
interdiff.txt1.01 KB
1852104-18.patch41.15 KB

Nitpicking here but a couple of the docblocks could be re-wrapped, they wrap early. Interdiff is against #16.

Status: Reviewed & tested by the community » Needs work
Issue tags: -API clean-up

The last submitted patch, 1852104-18.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Needs review
Issue tags: +API clean-up

#18: 1852104-18.patch queued for re-testing.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC. I reviewed the patch as well and agree with RTBC.

swentel’s picture

swentel CreditAttribution: swentel commented
FileSize
1852104-22.patch41.15 KB

Re-rolled to fix merge conflict in NodeFormController.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Alright, committed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

Updated issue summary.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Issue summary: View changes
Parent issue: » #1168246: Freedom For Fieldsets! Long Live The DETAILS.