(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/book/templates/book-all-books-block.html.twig
core/modules/book/templates/book-export-html.html.twig
core/modules/book/templates/book-navigation.html.twig
core/modules/book/templates/book-node-export-html.html.twig
core/modules/book/templates/book-tree.html.twig

Files: 
CommentFileSizeAuthor
#18 Screenshot-book-js-2014-11-12.png61.01 KBbradwade
#16 copy_book_templates_to-2349633-16.patch9.89 KBrunand
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,388 pass(es). View
#8 copy_book_templates_to-2349633-8.patch5.16 KBmortendk
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,636 pass(es). View
#4 book-to-classy-2349633-4.patch9.93 KBemma.maria
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,918 pass(es), 4 fail(s), and 0 exception(s). View
#2 classy-book.diff4.69 KBmortendk
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,871 pass(es), 4 fail(s), and 0 exception(s). View

Comments

davidhernandez’s picture

Issue summary: View changes
mortendk’s picture

Assigned: Unassigned » mortendk
Status: Active » Needs review
FileSize
4.69 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,871 pass(es), 4 fail(s), and 0 exception(s). View

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

emma.maria’s picture

Assigned: mortendk » emma.maria
Status: Needs review » Needs work
emma.maria’s picture

Assigned: emma.maria » Unassigned
Status: Needs work » Needs review
FileSize
9.93 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,918 pass(es), 4 fail(s), and 0 exception(s). View

The module template folder was added directly to Classy root folder, added them to classy/templates/book/*

Status: Needs review » Needs work

The last submitted patch, 4: book-to-classy-2349633-4.patch, failed testing.

The last submitted patch, 2: classy-book.diff, failed testing.

davidhernandez’s picture

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

mortendk’s picture

Issue tags: -banana +banana cssbanana
FileSize
5.16 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,636 pass(es). View

removing the id="book-navigation-{{ book_id }}" in book-navigation.html.twig gave me a bunch of test errors

cant find where n why but this should pass the test + move to /templates the discussion of where to put stuff is in #2349559: [meta] Discuss the organization of subfolders in Classy

mortendk’s picture

Status: Needs work » Needs review
Issue tags: -banana cssbanana +banana, +cssbanana

and testbot lets see what you got

mortendk’s picture

Issue tags: +drupalhagen
runand’s picture

Assigned: Unassigned » runand
runand’s picture

Assigned: runand » Unassigned
Status: Needs review » Reviewed & tested by the community

As mentioned, the id in book navigation is not removed, but everything else looks ok

lauriii’s picture

Status: Reviewed & tested by the community » Needs work

The templates are not generated to classy. Dont know whats wrong

lauriii’s picture

Status: Needs work » Reviewed & tested by the community

Ok, works while using git apply! Yeah!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/themes/classy/templates/book-node-export-html.html.twig
@@ -15,8 +15,8 @@
-<article id="node-{{ node.id }}" class="section-{{ depth }}">
-  <h1 class="book-heading">{{ title }}</h1>
+<article>
+  <h1>{{ title }}</h1>

Why are making these changes to the classy template? The classy template should have these classes no?

runand’s picture

Status: Needs work » Needs review
FileSize
9.89 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,388 pass(es). View
davidhernandez’s picture

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.

bradwade’s picture

While book.js does reference classes, specifically .book-outline-form and .book-title-select, neither of these were removed in these template changes. (Those classes are added in by BookManager.php)

I tested and the book.js continues to function properly with patch #16 applied. To test this you must enable Stark as your admin theme and add/edit a book page (book module must be enabled). The JS updates the summary under the heading of the "book outline" vertical tab when a new book/parent is selected. Note: This JS is not being executed in the Seven theme as Seven has removed this functionality of displaying a summary along with the vertical tab heading.

To summarize: Double checked. Javascript not broken.

mortendk’s picture

@bradwave Ok then we should set it back to RTBC :)

bradwade’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue is a prioritized change (theme improvements) as per https://www.drupal.org/core/beta-changes and it's benefits outweigh any disruption. Committed 8de9b67 and pushed to 8.0.x. Thanks!

  • alexpott committed 8de9b67 on 8.0.x
    Issue #2349633 by mortendk, runand, bradwade, emma.maria: Copy book...
mortendk’s picture

*high fives all around*

Status: Fixed » Closed (fixed)

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