Remove classes from system templates t*.html.twig [#2407745]

Problem/Motivation

Remove non essential classes from core - the classes are in classy, cores templates should be as clean as possible.
js required classes should be prefixed to js- according to code standards
simpletest should be done with classy theme, not stark

Proposed resolution

See parent issue(s) and related issue(s) for details,

Twig Templates to modify:

core/modules/system/templates/table.html.twig
core/modules/system/templates/tablesort-indicator.html.twig
~~core/modules/system/templates/task-list.html.twig~~
core/modules/system/templates/textarea.html.twig
core/modules/system/templates/time.html.twig

It appears task list no longer exists.

Remaining tasks

* test if resizable classes are essential
* test tablesort to still work

User interface changes

API changes

Comment	File	Size	Author
#23	table-before.png	249.37 KB	rteijeiro
#23	table-after.png	250.39 KB	rteijeiro
#23	form-before.png	466.4 KB	rteijeiro
#23	form-after.png	459.81 KB	rteijeiro
#22	t-templates-2.diff	3.16 KB	mortendk
#22
#20	t-templates.diff	3.16 KB	mortendk
#20
#18	text-area-resize.gif	400.27 KB	mortendk
#18	text-area-resize-seven.gif	176.07 KB	mortendk
#18	f-template-interdiff.txt	599 bytes	mortendk
#18	cleanup-templates-f-4.diff	3.81 KB	mortendk
#18
#16	cleanup-templates-f-3.diff	3.16 KB	mortendk
#16
#6	clean-templates-t.diff	2.45 KB	mortendk
#6
#1	issue-2407745-1.patch	8.08 KB	Sivaji_Ganesh_Jojodae
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented 15 January 2015 at 17:52

Status:

Active

» Needs review

File	Size
issue-2407745-1.patch	8.08 KB

Comment #2

15 January 2015 at 18:29

Status:

Needs review

» Needs work

The last submitted patch, 1: issue-2407745-1.patch, failed testing.

Comment #3

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented 15 January 2015 at 18:48

Line causing the test to fail is below,

$this->assertRaw('<tr class="odd"><td colspan="3" class="empty message">Empty row.</td>', 'Colspan on #empty row found.');

Wondering if I should update the template file or test case.

Comment #4

davidhernandez

English

USA

CreditAttribution: davidhernandez commented 19 January 2015 at 14:06

Status:

Needs work

» Postponed

Postponing this until we decide how we are moving forward. Keep an eye on #2348543: [meta] Consensus Banana Phase 2, transition templates to the starterkit theme for updates. Thanks.

Comment #5

davidhernandez

English

USA

CreditAttribution: davidhernandez commented 8 February 2015 at 23:34

Title:	Copy system templates t*.html.twig to Classy	» Remove classes from system templates t*.html.twig
Issue summary:	View changes
Status:	Postponed	» Needs work

Un-postponing. The templates have been copied to Classy, so all we need to do is remove classes from the original templates.

Comment #6

mortendk CreditAttribution: mortendk commented 10 February 2015 at 01:58

Status:

Needs work

» Needs review

File	Size
clean-templates-t.diff	2.45 KB

we need visual confirmation on resizable textareas still works without classes, else selector classes must be prefixed with .js-

Comment #7

10 February 2015 at 02:15

Status:

Needs review

» Needs work

The last submitted patch, 6: clean-templates-t.diff, failed testing.

Comment #8

10 February 2015 at 02:44

Status:

Needs work

» Needs review

davidhernandez queued 6: clean-templates-t.diff for re-testing.

Comment #9

10 February 2015 at 03:05

Status:

Needs review

» Needs work

The last submitted patch, 6: clean-templates-t.diff, failed testing.

Comment #10

mortendk CreditAttribution: mortendk commented 10 February 2015 at 09:21

Issue summary:

View changes

Comment #11

mortendk CreditAttribution: mortendk commented 10 February 2015 at 09:21

Issue summary:

View changes

Comment #12

mortendk CreditAttribution: mortendk commented 10 February 2015 at 09:22

Issue summary:

View changes

Comment #13

mortendk CreditAttribution: mortendk commented 10 February 2015 at 09:23

Issue summary:

View changes

Comment #14

mortendk CreditAttribution: mortendk commented 14 February 2015 at 23:05

Issue tags:

+Needs screenshots

Comment #15

mortendk CreditAttribution: mortendk commented 15 February 2015 at 11:21

Issue tags:

+test on classy

function testThemeTableWithEmptyMessage() in tableTest.php needs to test on classy not on stark

Comment #16

mortendk CreditAttribution: mortendk commented 15 February 2015 at 11:28

Status:

Needs work

» Needs review

File	Size
cleanup-templates-f-3.diff	3.16 KB

fixed the test

Comment #17

mortendk CreditAttribution: mortendk commented 15 February 2015 at 11:31

Issue summary:

View changes

Comment #18

mortendk CreditAttribution: mortendk commented 15 February 2015 at 11:47

Issue summary:

View changes

File	Size
cleanup-templates-f-4.diff	3.81 KB

f-template-interdiff.txt	599 bytes
text-area-resize-seven.gif	176.07 KB
text-area-resize.gif	400.27 KB

screenshots added of resize

Removed text-area class that was set from the module : housecleaning

Comment #19

15 February 2015 at 12:10

Status:

Needs review

» Needs work

The last submitted patch, 18: cleanup-templates-f-4.diff, failed testing.

Comment #20

mortendk CreditAttribution: mortendk commented 15 February 2015 at 14:54

Status:	Needs work	» Needs review
Issue tags:	-cssbanana, -Needs screenshots, -test on classy

File	Size
t-templates.diff	3.16 KB

Reuploaded patch from #16 - im an idiot so it was named as beeing F* templates, so lets try with the right letter

One issue: .form-textarea class is still added to the textarea, when using stark.
Looks like it gets the name from form.inc

function template_preprocess_textarea(&$variables) {
  $element = $variables['element'];
  Element::setAttributes($element, array('id', 'name', 'rows', 'cols', 'placeholder'));
  Element\RenderElement::setAttributes($element, array('form-textarea'));
  $variables['wrapper_attributes'] = new Attribute();

Dunno why this is added but its there.
We could make a cowboy solution and use {{ attributes.removeClass('form-textarea') }} on the core template, but thats to ugly even for sloppy themers ;)

Anybody got a clue on why & where this comes from or should be make it into a followup issue & get the templates cleaned up now ?

Comment #21

15 February 2015 at 15:43

Status:

Needs review

» Needs work

The last submitted patch, 20: t-templates.diff, failed testing.

Comment #22

mortendk CreditAttribution: mortendk commented 15 February 2015 at 16:19

Status:

Needs work

» Needs review

File	Size
t-templates-2.diff	3.16 KB

lets try thins again

Comment #23

rteijeiro CreditAttribution: rteijeiro commented 16 February 2015 at 17:04

Status:

Needs review

» Reviewed & tested by the community

File	Size
form-after.png	459.81 KB
form-before.png	466.4 KB
table-after.png	250.39 KB
table-before.png	249.37 KB

Good job, @mortendk! Yeah!

Table BEFORE

Table AFTER

Form BEFORE

Form AFTER

Comment #24

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott commented 17 February 2015 at 15:02

Status:

Reviewed & tested by the community

» Fixed

Templates are not frozen in beta. Committed 275947d and pushed to 8.0.x. Thanks!

Comment #25

17 February 2015 at 15:03

alexpott committed 275947d on 8.0.x

Issue #2407745 by mortendk, rteijeiro, sivaji@knackforge.com: Remove...

Comment #26

3 March 2015 at 15:04

Status:

Fixed

» Closed (fixed)

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

Remove classes from system templates t*.html.twig

Problem/Motivation

Proposed resolution

Remaining tasks

User interface changes

API changes

Comments

Table BEFORE

Table AFTER

Form BEFORE

Form AFTER

Change records for this issue

Parent issue

Related issues