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

Files: 
CommentFileSizeAuthor
#4 2473957-text-forms-split-4.patch4.77 KBsqndr
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,505 pass(es). View
#4 interdiff.txt421 bytessqndr
#1 2473957-text-forms-split.patch4.49 KBCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,633 pass(es). View

Comments

Cottser’s picture

Status: Active » Needs review
FileSize
4.49 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 91,633 pass(es). View

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

LewisNyman’s picture

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');
        }
Cottser’s picture

Excellent, thanks Lewis!

sqndr’s picture

FileSize
421 bytes
4.77 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 92,505 pass(es). View

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

Issue tags: +Novice
sqndr’s picture

Status: Needs work » Needs review
LewisNyman’s picture

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

working fine for me :)

alexpott’s picture

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...
Cottser’s picture

Woo! Thanks all :)

Status: Fixed » Closed (fixed)

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