Task

Prefix text-* classes with js- to separate classes needed for JavaScript functionality from those used for styling and make it clear which classes can safely be removed without breaking JavaScript functionality.

Affected classes:

  • text-format-wrapper
  • text-full
  • text-summary
  • text-summary-wrapper

Remaining tasks

  • Patch
  • Patch review
  • Manual testing

Steps to test

  1. Navigate to node/add/article
  2. Click on the "Edit Summary" link
  3. Click on the "Hide Summary" link

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task
Issue priority Normal
Unfrozen changes Unfrozen because it mostly just affects templates and JS which are not frozen.
Prioritized changes The main goal of this issue is to improve themer experience and arguably it reduces fragility where JavaScript and markup intersect.
Disruption Shouldn't be too disruptive as it is mostly affecting CSS classes that are added to markup. Themes extending Classy will only have classes added. Themes not extending Classy will have classes removed but they can be added back via template overrides.

User interface changes

None for themes extending Classy. Possible visual changes for other themes.

API changes

n/a

CommentFileSizeAuthor
#4 2473957-text-forms-split-4.patch4.77 KBsqndr
#4 interdiff.txt421 bytessqndr
#1 2473957-text-forms-split.patch4.49 KBstar-szr

Comments

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 2473957-text-forms-split.patch4.49 KB

Patch split from parent issue. I wasn't able to find anything additional by grepping.

lewisnyman’s picture

lewisnyman
he/him
commented
Issue summary: View changes
Status: Needs review » Needs work

I grepped and I found one more find function in text.js:

        // If no summary is set, hide the summary field.
        if ($widget.find('.text-summary').val() === '') {
          $link.trigger('click');
        }
star-szr’s picture

star-szr
he/him
Primary language English
commented

Excellent, thanks Lewis!

sqndr’s picture

sqndr commented
StatusFileSize
new interdiff.txt421 bytes
new 2473957-text-forms-split-4.patch4.77 KB

I grepped and I found one more find function in text.js:

I went on and wrote a patch for that. Tested on node/add/article and "Edit Summary" & "Hide Summary" still works.

sqndr’s picture

sqndr commented
Issue tags: +Novice
sqndr’s picture

sqndr commented
Status: Needs work » Needs review
lewisnyman’s picture

lewisnyman
he/him
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing

Ace, I manually tested this patch and it stills works. Thanks!

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

working fine for me :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Category: Task » Bug report
Status: Reviewed & tested by the community » Fixed

This is actually fixing a bug since... 

+++ b/core/modules/filter/templates/text-format-wrapper.html.twig
@@ -15,7 +15,7 @@
-<div>
+<div class="js-text-format-wrapper">

... the default filter template had had the classes removed.

Committed d4bef05 and pushed to 8.0.x. Thanks!

  • alexpott committed d4bef05 on 8.0.x 
    Issue #2473957 by sqndr, Cottser, LewisNyman: Prefix text-* classes with...
star-szr’s picture

star-szr
he/him
Primary language English
commented

Woo! Thanks all :)

Status: Fixed » Closed (fixed)

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