@czolnowski opened this Pull Request on June 18th 2014 Contributor

Adding this line gave higher priority for third-party theme, but doesn't cause problems described in this commit:
https://github.com/piwik/piwik/commit/b06b1189c355ec8ecc9dc912f30da55432b9a126

Maybe this will fix bug with third-party themes! :)

@mattab commented on June 19th 2014 Member

as far as I understand the themes inherit from Morpheus so Morpheus should be loaded first and then other themes should be loaded. If there is a good reason to load current theme before Morpheus, maybe you can give me an example?

@czolnowski commented on June 19th 2014 Contributor

As I tested - this change should prioritise third-party theme which is currently active and move this theme less file at the end of final css file.

I'll test this again and confirm this behavior. Please, don't close this PR before my confirmation.

@mattab commented on June 19th 2014 Member

Sounds good! Have a good holiday
related ticket is http://dev.piwik.org/trac/ticket/5345

@mattab commented on June 24th 2014 Member

@czolnowski any update? maybe this needs to go in 2.4.0 so looking forward to it.

@czolnowski commented on June 24th 2014 Contributor

https://www.dropbox.com/s/orxak0l8fdutrri/323_PR.png
@mattab: Please, take a look at this screen. It's xdebug breakpoint from UIAssetCatalogSorter:46. In this line you have sorted and ready to merge less files. First console shows files order before PR and second after PR. On second console you can see that custom theme (PROClean) is at the end of files array. Everything looks great, but... This is theme from piwik 2.3.x. And it's overwriting all files from Morpheus. I'm not sure how it will works without these overwriting.
Morpheus theme file should be on index 50, but I'm not sure why piwik put them on index ~5. (it is starting index for morpheus on my instance, so maybe piwik doesn't recognize this in sorting process?)
I'm open for every suggestion. Would be great if this change will be available in 2.4.0 release!

@mattab commented on June 24th 2014 Member

Are you seeing the custom theme loaded before Morpheus? That would be a bug.

If the custom theme is loaded after Morpheus, then it's working.

@czolnowski commented on June 24th 2014 Contributor

"Are you seeing the custom theme loaded before Morpheus? That would be a bug." - this is behavior in current master.

"If the custom theme is loaded after Morpheus, then it's working." - this is behavior with my PR applied.

@quba commented on June 24th 2014 Contributor

But one thing to improve would be to move Morpheus just one step before custom theme. Now I think it's incorrect, because Morpheus loads before other Piwik CSS files.

@czolnowski commented on June 24th 2014 Contributor

@mattab: Please, take a look now. Morpheus and custom theme working together. If custom theme is enabled then order is: Morpheus -> Custom theme. And this is backward compatible.

@mattab commented on June 24th 2014 Member

Cheers I've tested quickly and looks correct. I made smalll suggestion: https://github.com/czolnowski/piwik/pull/2
please merge and then i'll merge this one!

@czolnowski commented on June 25th 2014 Contributor
@mattab commented on June 26th 2014 Member

Ok I understand now... LESS compiler does not work as we expected http://stackoverflow.com/questions/15383869/overriding-bootstrap-less-variables-after-import

Maybe we have to fork and extend, or inherit the lessC compiler? The new feature would be to allow defining variables in later content, and that the last variable value would be used.

For this maybe we could detect all variables used in any of the files and imported files, then move those variables up and first in the Less merged file, and then compile the less file and less would replace variable names by their latest defined value. Thoughts?

@mattab commented on June 30th 2014 Member

Here is my test procedure:

  • create a theme with: ./console generate:theme
  • in your theme/stylesheets/_colors.less set:
<a class='mention' href='https://github.com/theme'>@theme</a>-color-brand:                    <a href='/000'>#000</a>;
<a class='mention' href='https://github.com/theme'>@theme</a>-color-text:                     <a href='/000'>#000</a>;
<a class='mention' href='https://github.com/theme'>@theme</a>-color-link:                     <a href='/000'>#000</a>;
  • delete cached styles $ rm tmp/assets/* -f
  • activate theme ./console core:plugin activate Testtheme2
  • in config file I put the Plugins[]=Testtheme2 before Plugins[]=Morpheus
  • I load Piwik and the colors overrides are applied

This works without and with the pull request.

@mattab commented on June 30th 2014 Member

Ok I understand now... LESS compiler does not work as we expected

well actually... @tsteur said that when we load the theme files last, it should work and overwrite all variables defined before. If you find that this is not true please send us a test theme and steps to reproduce the issue and we will investigate issue.

This Pull Request was closed on June 30th 2014
Powered by GitHub Issue Mirror