Issue #2473941 by mortendk, rteijeiro, rachel_norfolk, Cottser, LewisNyman: Prefix field-parent classes with js-

Task

Prefix field-parent 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.

Remaining tasks

  • Patch
  • Patch review
  • Manual testing

Steps to test

@todo

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
#1 interdiff-2473941-field-parent.txt596 bytesstar-szr
#1 2473941-field-parent-additional.patch1.83 KBstar-szr
#1 2473941-field-parent-split.patch1.25 KBstar-szr
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Active » Needs review
FileSize
2473941-field-parent-split.patch1.25 KB
2473941-field-parent-additional.patch1.83 KB
interdiff-2473941-field-parent.txt596 bytes

Initial patch split from the parent, some additional changes from grepping, and interdiff between the two.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue tags: +banana
LewisNyman’s picture

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

I manually tested this patch and the field ui works as it did before. I also had a quick grep for any remaining field-parent in Javascript and there was none.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: 2473941-field-parent-additional.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Reviewed & tested by the community

Random fail:

Drupal\simpletest\Tests\SimpleTestBrowserTest - "0 fails, 0 exceptions" found

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Needs work

If this class is used for js, why is no js updated in the patch?

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Needs review

The -additional is the one to review, it just succumbed to a random test fail :/

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr at Northern Commerce commented
Issue summary: View changes

Adding suggested commit message.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 07bcb91 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation.

  • alexpott committed 5419c8d on 8.0.x 
    Issue #2473941 by mortendk, rteijeiro, rachel_norfolk, Cottser,...

Status: Fixed » Closed (fixed)

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