@diosmosis opened this Pull Request on July 30th 2018 Member

Changes:

  • Add new event Emails.setThemeVariables that modifies the variables used to set the fonts/colors in emails. Currently internal. Used by ExampleTheme.
  • Change twig templates to use hex colors where it was using rgb colors before (apparently hex colors are better supported in email clients).

An email w/ ExampleTheme settings & a serif font:

image

Fixes #12919

@mattab commented on August 1st 2018 Member

Noticed the color of Table of content regressed in the branch (it used to be darker grey):

screenshot from 2018-08-02 01-48-21

@mattab commented on August 1st 2018 Member

The main use case for this issue/PR is to be compatible with White Label. Just tested to configure WhiteLabel like this:

screenshot from 2018-08-02 01-55-00

and the HTML email report is un-changed.

-> was it supposed to work maybe?

@diosmosis commented on August 2nd 2018 Member

Created a WhiteLabel PR, here is a pic:

image

BTW, original issue should mention WhiteLabel if that is the main reason for it.

@mattab commented on August 2nd 2018 Member

BTW, original issue should mention WhiteLabel if that is the main reason for it.

oops, sorry forgot to mention this!

LGTM, just the tests needs fixing (minor changes in the html/pdf reports)

@tsteur commented on August 3rd 2018 Member

As mentioned in other PR eventually be good to

1) Use an object instead of an array for getDefaultEmailStyles so auto completion and refactoring etc works . Like class EmailStyles { public $brandNameLong = ''; }

2) Eventually we should define theme colors only in one place. Now a theme designer needs to set it in php plus less. Ideally we would only do this in PHP. There could be one Theme object to configure theme colors like class ThemeConfiguration { public $colorText = ''; public $colorLink = ''; } and then the class EmailStyles would remove quite a few properties and use where possible those colors. There wouldn't need to be any theme.less anymore as all colors would be possible to configure through PHP in one place. PHP would then add the less theme variables through AssetManager.addStylesheets event.

I think this should be done for consistency maybe at the latest in Matomo 4 or 5, or now if not too much work. Could be quick. FYI @mattab @diosmosis

@diosmosis commented on August 3rd 2018 Member

Could add these to new issue(s) for prioritization.

@mattab commented on August 5th 2018 Member

@diosmosis sounds good, could you create a new issue in 3.7.0?

@diosmosis commented on August 5th 2018 Member

will do

@diosmosis commented on August 6th 2018 Member
This Pull Request was closed on August 2nd 2018
Powered by GitHub Issue Mirror