search

plone / Products.CMFPlone

The core of the Plone content management system
https://plone.org
GNU General Public License v2.0
240 stars 183 forks source link

Use details element for collapsibles in the resource registry #3976

Closed reinhardt closed 2 weeks ago

reinhardt commented 3 weeks ago

Makes it possible to toggle elements even with broken or missing javascript.

However, this changes the look & feel. The CSS is not applied any more because some classes were removed so that the accordion JS does not interfere. Maybe there's a better way to do this, or maybe we just need some additional CSS?

2024-06-15-12-58-42_2552x1368

A note on accessibility: A details/summary combination is already accessible and aparently does not need any more aria attributes. However, the <summary> should be descriptive enough, we could argue if it is in our case. But I believe, thati's fine already. More info see: http://kb.daisy.org/publishing/docs/html/details.html

Fixes #3942

PRs: https://github.com/plone/Products.CMFPlone/pull/3976 https://github.com/plone/plonetheme.barceloneta/pull/374

mister-roboto commented 3 weeks ago

@reinhardt thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

rohnsha0 commented 3 weeks ago

@jenkins-plone-org please run jobs

mauritsvanrees commented 3 weeks ago

+1 from me. A few tests are failing though.

thet commented 2 weeks ago

I've added some classes and styles. It now looks almost like before:

image

PR: https://github.com/plone/plonetheme.barceloneta/pull/374

thet commented 2 weeks ago

Now with label wrapping their inputs and no id's being duplicated (tnx @ale-rt for pointing this out).

Together with the changes in plonetheme.barceloneta, this looks like that:

image

reinhardt commented 2 weeks ago

Awesome, thanks @thet! LGTM (Though I can't give an approving review because I created the PR :stuck_out_tongue:)

thet commented 2 weeks ago

@petschki did you also check out the plonetheme.barceloneta branch from https://github.com/plone/plonetheme.barceloneta/pull/374 ?

For me, it looks like this:

image

thet commented 2 weeks ago

@jenkins-plone-org please run jobs