Problem/Motivation

Styles from css/components/tour.theme.css is loaded on all seven theme pages independently of tour module enabled state.

Remaining tasks

Load tour style theme modifications only when tour exposes it's library

User interface changes

None

API changes

None

Data model changes

None

Comments

LewisNyman created an issue. See original summary.

irina.rozite’s picture

FiIe tour.theme.css has been already moved to the Seven theme. I assume it was done in some other issue, but probably file should be renamed to tour.css

irina.rozite’s picture

Status: Active » Needs review
StatusFileSize
new2.98 KB

Added patch

Status: Needs review » Needs work

The last submitted patch, 3: move_theme_tour-2566855-3.patch, failed testing.

The last submitted patch, 3: move_theme_tour-2566855-3.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: move_theme_tour-2566855-3.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: move_theme_tour-2566855-3.patch, failed testing.

hog’s picture

Rerroled patch.

hog’s picture

Status: Needs work » Needs review

Rerroled patch.

Status: Needs review » Needs work

The last submitted patch, 10: move_theme_tour-2566855-10.patch, failed testing.

andypost’s picture

+++ b/core/modules/tour/tour.module
@@ -44,7 +86,7 @@ function tour_toolbar() {
-  $items['tour'] += array(
+  $items['tour'] = array(

that hardly overrides, are you sure that needed?

hog’s picture

Status: Needs work » Needs review
StatusFileSize
new427 bytes
new564 bytes

Status: Needs review » Needs work

The last submitted patch, 14: move_theme_tour-2566855-14.patch, failed testing.

hog’s picture

Status: Needs work » Needs review
StatusFileSize
new7.79 KB
new7.79 KB

Re-rolled path.

hog’s picture

StatusFileSize
new7.94 KB

The last submitted patch, 16: move_theme_tour-2566855-16.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 16: move_theme_tour-2566855-16.patch, failed testing.

hog’s picture

Status: Needs work » Needs review
StatusFileSize
new7.38 KB
new513 bytes

Applayed patch for new 8.0.x

Status: Needs review » Needs work

The last submitted patch, 20: move_theme_tour-2566855-20.patch, failed testing.

andypost’s picture

Component: quickedit.module » Seven theme
Status: Needs work » Needs review
StatusFileSize
new1.09 KB

Different approach - tour theme should be added from seven theme only when tour module enabled

Related CR https://www.drupal.org/node/2497313

  1. +++ b/core/modules/tour/tour.module
    @@ -8,6 +8,48 @@
    +function tour_library_info_alter(&$libraries, $extension) {
    

    the module should not care about themes at all

  2. +++ b/core/themes/seven/seven.libraries.yml
    @@ -29,7 +29,7 @@ global-styling:
    -      css/components/tour.theme.css: {}
    +      css/components/tour.css: {}
    

    I think this file should be attached conditionally when tour module enabled
    so better to use hook_library_info_alter() in seven theme instead of library_extend

hog’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new260.02 KB

Styles moved to seven theme, all ok as i see.
moved-tour-css

star-szr’s picture

This could use a title and issue summary update since it's not really doing what is described, it's just changing Seven's tour CSS to load more conditionally.

andypost’s picture

Title: Move theme tour CSS to the Seven theme » Load tour CSS in the Seven theme only when the module enabled
Issue summary: View changes

  • Cottser committed 8d48a88 on
    Issue #2566855 by HOG, andypost, irina.pusko: Load tour CSS in the Seven...
star-szr’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs issue summary update

Since this is an optimization of when the tour CSS is loaded I don't think it makes sense to commit to 8.0.x. Also, the risk is low in committing to 8.1.x because the only selectors in Seven's tour.theme.css are from joyride.

Committed 8d48a88 and pushed to 8.1.x. Thanks!

wim leers’s picture

Yes! libraries-extend saves the day once again :)

Status: Fixed » Closed (fixed)

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