Issue #2473953 by mortendk, rteijeiro, rachel_norfolk, Cottser, LewisNyman: Prefix form-submit classes with js-

Task

Prefix form-submit 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

  1. Navigate to the node/add/article
  2. Add an image to the upload field
  3. Remove the image

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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

star-szr’s picture

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

LewisNyman’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing

I manually tested the patch, steps in the issue summary. I also grepped for form-submit and couldn't find anything else to change.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: 2473953-form-submit-additional.patch, failed testing.

star-szr’s picture

Status: Needs work » Reviewed & tested by the community

Failed because d.o was down. Thanks Lewis!

catch’s picture

Status: Reviewed & tested by the community » Needs review

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.

This isn't the case. Stark ends up with both classes.

star-szr’s picture

Sure, if those classes are added in preprocess. In that case it's out of our hands unless we can and want to move the classes to templates.

star-szr’s picture

Issue summary: View changes

Adding suggested commit message.

LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Makes sense to me

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Which patch is rtbc?

star-szr’s picture

Status: Needs work » Needs review
FileSize
3.72 KB

Re-uploading the relevant patch.

aburrows’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
185.78 KB
155.97 KB

Tested patch and works as intended

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed d8442c8 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed d8442c8 on 8.0.x
    Issue #2473953 by mortendk, rteijeiro, rachel_norfolk, Cottser, aburrows...

Status: Fixed » Closed (fixed)

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