(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/aggregator/templates/aggregator-block-item.html.twig
core/modules/aggregator/templates/aggregator-feed.html.twig
core/modules/aggregator/templates/aggregator-item.html.twig

CommentFileSizeAuthor
#12 copy_aggregator-2349615-12.patch2.11 KBmortendk
#10 copy_aggregator-2349615-10.patch2.18 KBmortendk
#8 copy_aggregator-2349615-8.patch2.18 KBmortendk
#4 classy-aggregator.diff2.12 KBmortendk

Comments

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Issue summary: View changes
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Issue summary: View changes
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Issue summary: View changes
mortendk’s picture

mortendk commented
Issue summary: View changes
StatusFileSize
new classy-aggregator.diff2.12 KB

moved the classes out of core
Discussion of the classes - should be done as a follow up, so bikesheedding wont stop classy ;)

mortendk’s picture

mortendk commented
Status: Active » Needs review
mortendk’s picture

mortendk commented
Assigned: Unassigned » mortendk

Status: Needs review » Needs work

The last submitted patch, 4: classy-aggregator.diff, failed testing.

mortendk’s picture

mortendk commented
Status: Needs work » Needs review
StatusFileSize
new copy_aggregator-2349615-8.patch2.18 KB

moved the files into classy/templates

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Status: Needs review » Needs work

These templates have been moved to classy/templates/aggregator not classy/templates/

mortendk’s picture

mortendk commented
Status: Needs work » Needs review
StatusFileSize
new copy_aggregator-2349615-10.patch2.18 KB

*doh* now the templates are moved to /classy/templates/aggregator/

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
Status: Needs review » Needs work

What I meant was the templates should be in classy/templates, not classy/templates/aggregator.

mortendk’s picture

mortendk commented
Status: Needs work » Needs review
StatusFileSize
new copy_aggregator-2349615-12.patch2.11 KB

moved templates into /templates - no subfolders

mortendk’s picture

mortendk commented
Issue tags: +drupalhagen
danquah’s picture

danquah commented
Assigned: mortendk » danquah
danquah’s picture

danquah commented
Status: Needs review » Reviewed & tested by the community

Confirmed, 3 templates ended up in core/themes/classy/templates/ all classes have been stripped from core/modules/aggregator/templates/aggregator-*

danquah’s picture

danquah commented
Assigned: danquah » Unassigned
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented
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

mortendk commented
Status: Needs review » Reviewed & tested by the community

aggregator dont have any classes thats used by css - setting it back to rtbc (if i can do that ?)

alexpott’s picture

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

I can confirm that the classes are not used in css or js.

Committed 1fea878 and pushed to 8.0.x. Thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

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 1fea878 on 8.0.x 
    Issue #2349615 by mortendk | davidhernandez: Copy aggregator templates...

Status: Fixed » Closed (fixed)

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