(First, verify that the preprocess changes have been made. #2322163: [meta] Consensus Banana Phase 1, move CSS classes from preprocess to twig templates.)

  1. Copy the Twig templates from the core module's templates directory to Classy's templates directory. Include all templates, even ones without classes.
  2. Remove all classes from the core module's template. Remove all classes added with addClass and ones that are hard-coded in the template.
  • If there are classes that are required for basic functionality, discuss whether they should be kept.
  • If there is CSS from the module, or anywhere else, referring to the class, discuss removing it or moving it to Bartik&Seven. Do not move the CSS to Classy.

Twig Templates to Copy

core/modules/taxonomy/templates/taxonomy-term.html.twig

Files: 
CommentFileSizeAuthor
#29 taxonomy-quickfix.patch2.93 KBlauriii
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,682 pass(es). View
#28 taxonomy-quickfix.diff1.48 KBmortendk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,696 pass(es). View
#27 2349765-taxonomy-20.patch.txt2.41 KBmortendk
#20 2349765-taxonomy-20.patch2.41 KBderheap
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,656 pass(es). View
#9 2349765-9.patch1.62 KBderheap
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,647 pass(es). View
#7 2349765-7.patch1.66 KBderheap
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,860 pass(es). View
#1 2349765-1.patch1.62 KBderheap
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,764 pass(es), 3 fail(s), and 0 exception(s). View

Comments

derheap’s picture

FileSize
1.62 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,764 pass(es), 3 fail(s), and 0 exception(s). View

Removed classes and ids.
Copied the template to classy/templates/taxonomy.

There are some lines of CSS in the module, but they seem to be needed by the taxonomy overview form, so I left them alone.

derheap’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: 2349765-1.patch, failed testing.

Status: Needs work » Needs review

derheap queued 1: 2349765-1.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2349765-1.patch, failed testing.

davidhernandez’s picture

+++ b/core/modules/taxonomy/templates/taxonomy-term.html.twig
@@ -14,11 +14,7 @@
  * - attributes: HTML attributes for the wrapper. The 'class' attribute
- *   contains the following classes by default:
- *   - taxonomy-term: The current template type, i.e. "theming hook".
- *   - vocabulary-[vocabulary-name]: The vocabulary that this term belongs to.
- *     For example, if the term belongs to a vocabulary called "Tags" then the
- *     class would be "vocabulary-tags".
+ *   contains no classes by default.

I don't think we need to comment that there are no classes by default.

Also, the blank line is not needed on line 30 before the div where the term id is removed.

derheap’s picture

Status: Needs work » Needs review
FileSize
1.66 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,860 pass(es). View

Made the changes according to #6.
And we have to keep the attributes on the div ( <div{{ attributes}}>).
Otherwise RDF-Support (and maybe other modules) break.

davidhernandez’s picture

Status: Needs review » Needs work

The templates should go into the root of the templates folder, not a subfolder.

derheap’s picture

Status: Needs work » Needs review
Issue tags: +cssbanana
FileSize
1.62 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,647 pass(es). View

OK. Moved to the template to the root template folder.

Tagging cssbanana, as there are some lines of css, which might be needed for admin.
We should investigate in a followup.

mortendk’s picture

Issue tags: +drupalhagen
Xen’s picture

Assigned: Unassigned » Xen
Xen’s picture

New template file is properly used.

Xen’s picture

Assigned: Xen » Unassigned
Status: Needs review » Reviewed & tested by the community
davidhernandez’s picture

Status: Reviewed & tested by the community » Needs review

I'm universally setting all the phase 2 issues back to needs review, because we're missing some things. Please double-check if any removed classes are being used in javascript. It is best to test the affected template using Stark to make sure nothing is broken.

mortendk’s picture

Status: Needs review » Reviewed & tested by the community

no .js is effecting the taxonomy terms templates.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/taxonomy/templates/taxonomy-term.html.twig
@@ -13,12 +13,7 @@
- * - attributes: HTML attributes for the wrapper. The 'class' attribute
- *   contains the following classes by default:
- *   - taxonomy-term: The current template type, i.e. "theming hook".
- *   - vocabulary-[vocabulary-name]: The vocabulary that this term belongs to.
- *     For example, if the term belongs to a vocabulary called "Tags" then the
- *     class would be "vocabulary-tags".
+ * - attributes: HTML attributes for the wrapper.

This fix is also applicable to the template being copying to classy. Let's keep them in sync.

Apart from that I can confirm the the classes added here are not used in any css or js.

derheap’s picture

OK, I will fix in the classy template too.

Can I remove the css classes, as they are not used?

alexpott’s picture

I don't think we should be removing the classes. But I'll leave that to the likes of @davidhernandez, @john_albin or @mortendk to make the call.

davidhernandez’s picture

@derheap, are you referring to the classes in the Classy copy of the template? If so, no, those classes stay. The only classes that would get removed are in the template kept in the taxonomy module.

derheap’s picture

Status: Needs work » Needs review
FileSize
2.41 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,656 pass(es). View

Fixed classy template according to #16.

@davidhernandez
Yes, I was over ambitious. I left them in.

mortendk’s picture

Status: Needs review » Reviewed & tested by the community

looks good to me. and yes we fist begin to hammer on classes when they are in classy

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 0ba69d7 and pushed to 8.0.x. Thanks!

This normal task was committed due the fact the banana consensus received per approval wrt to #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?

  • alexpott committed 0ba69d7 on 8.0.x
    Issue #2349765 by derheap | davidhernandez: Copy taxonomy templates to...
dcrocks’s picture

Why wasn't taxonomy-term.html.twig placed in the 'templates' sub-directory of classy, as the others were?

davidhernandez’s picture

There shouldn't be any templates going into subdirectories. We will address that later. See #2349559: [meta] Discuss the organization of subfolders in Classy

davidhernandez’s picture

Status: Fixed » Needs work
Issue tags: +Quick fix

Oh I see. That went into the root. That's a problem. We need a quick fix.

mortendk’s picture

Status: Needs work » Needs review
FileSize
2.41 KB

quickfix moving the taxonomy-term.html.twig into /templates

mortendk’s picture

FileSize
1.48 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,696 pass(es). View

*sigh* lets try again with the correct file

lauriii’s picture

FileSize
2.93 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 80,682 pass(es). View
alexpott’s picture

Status: Needs review » Fixed
Issue tags: -Quick fix

I'm no longer committing on-issue followups since there cause issue management problems. For example, this is now a bug and not a task. Please open a new issue to do the move.

lauriii’s picture

Status: Fixed » Closed (fixed)

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