Problem/Motivation

#3111124: Force inclusion of drupal.collapse shim added the Drupal core shim to support details in older browsers. However, the shim expects specific markup so the content can be opened and closed when clicking the summary.

Proposed resolution

Add the html markup that is expected by drupal/collapse so older browsers can open and close content of the details element.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

Older browsers can open and close content of the details element.

API changes

None.

Data model changes

The ckeditor details plugin requires div[class="details-wrapper"] to be allowed in the html filter.

CommentFileSizeAuthor
#8 3159065-8.patch4.87 KBLendude
#8 interdiff-3159065-7-8.txt647 bytesLendude
#7 3159065-7.patch4.85 KBLendude
#7 interdiff-3159065-6-7.txt396 bytesLendude
#6 3159065-5.patch4.91 KBidebr
#3 interdiff-2-3.txt1.68 KBidebr
#3 3159065-3.patch6.28 KBidebr
#2 3159065-2.patch5.66 KBidebr
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

idebr created an issue. See original summary.

idebr’s picture

idebr CreditAttribution: idebr at iO commented
Status: Active » Needs review
FileSize
3159065-2.patch5.66 KB

Attached patchs adds the html markup that is expected by drupal/collapse so older browsers can open and close content of the details element.

idebr’s picture

idebr CreditAttribution: idebr at iO commented
FileSize
3159065-3.patch6.28 KB
interdiff-2-3.txt1.68 KB

Attached patch adds the following changes:

  1. The summary element maintains a text element when all the text has been removed, so the user can enter a new summary text
  2. The details-wrapper is added retroactively for existing content.
mandclu’s picture

mandclu CreditAttribution: mandclu at Northern Commerce commented

Thanks for your work on this. I'll have to merge this in with some other work on the module lately, but that should be manageable. One question: You add the additional markup in the template definition, but I believe this only changes the markup within the editor. If the main aim is to provide the markup for visitors (presumably that's when the shim is most likely to be needed) then I would think the change needs to be made to the theme function (which your patch appears to remove entirely).

idebr’s picture

idebr CreditAttribution: idebr at iO commented

#4 I think you are referring to the Drupal.theme.detail method? I could not find any calls to this code, so I removed it.

idebr’s picture

idebr CreditAttribution: idebr at iO commented
FileSize
3159065-5.patch4.91 KB

Reroll for latest HEAD.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
FileSize
interdiff-3159065-6-7.txt396 bytes
3159065-7.patch4.85 KB

Adding the 'allowed content' killed the ability to add nested details (among other things). Removed the 'allowed content' for the content field.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
FileSize
interdiff-3159065-7-8.txt647 bytes
3159065-8.patch4.87 KB

#7 was adding additional wrappers, this should fix that.

  • mandclu committed 7429598 on 2.x authored by Lendude 
    Issue #3159065 by idebr, Lendude: Wrap details content in a div.details-...
mandclu’s picture

mandclu CreditAttribution: mandclu commented
Status: Needs review » Fixed

I believe this was included in the code already merged as part of #3201638: Entering an accordion title the spacebar does not produce the expected space so marking this as fixed.

Status: Fixed » Closed (fixed)

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