SparkDevNetwork / Rock

An open source CMS, Relationship Management System (RMS) and Church Management System (ChMS) all rolled into one.
http://www.rockrms.com
576 stars 350 forks source link

System Communications w/CSS Inlining Enabled Do Not Have Inline Styling #4631

Closed briankalwat closed 3 years ago

briankalwat commented 3 years ago

Prerequisites

Description

Communication Templates and System Communications in Rock have an option to enable CSS inlining on them (see below). We've recently implemented a block lava shortcode that handles the "frame" of all of our email templates, and we are wanting to use this same shortcode for communication templates as well as system communications.

As of (at least) 13.0.5, it appears the CSS inlining feature for system communications is not working in the same manner that it is for communications sent with the wizard. I'm unsure if this is an issue with CSS inlining for system communications as a whole, or if it's because we're using a shortcode.

Regular Communication Template Sent w/Communication Wizard

Notice the style definitions that have been added to the style attributes on the elements in the inspector window on the right.

image

System Communication

Notice the lack of style attributes on the elements in the inspector window on the right.

image

Steps to Reproduce

  1. Add the following Lava Shortcode to your Rock instance.
  2. Create a new communication template using the shortcode, and enable CSS inlining for it.
  3. Modify any system communication to use the lava shortcode you created in step 1, and enable CSS inlining for it.
  4. Send a communication from the communication wizard using the template you created in step 2.
  5. Trigger a send of the system communication you modified in step 3.

Expected behavior:

Both methods of sending communications should write the styles from the lava shortcode to the style attributes of the HTML elements themselves.

Actual behavior:

Only communications sent with the email wizard have their styles inlined properly.

Versions

briankalwat commented 3 years ago

To update this issue - CSS inlining is also not happening when the shortcode is used in the context of the Send Communication workflow action.

jonedmiston commented 3 years ago

@briankalwat can you confirm that you had the shortcode working previously in an older version.

briankalwat commented 3 years ago

@jonedmiston I was discussing with some other teammates and this is not limited to templates with shortcodes - we were having the issue with static system communications, as well. We started noticing the issue when we switched from Mandrill to Mailgun. From what we've found in researching this, Mandrill has a built-in css inliner that every message gets run through, while Mailgun does not (at least that I am aware of).

jonedmiston commented 3 years ago

@briankalwat You are correct the Mandrill did have an option in their API to inline while Mailgun does not. This inlining is done in their backend.

There's the possibility of adding inlining as a core feature using something like PreMailer. We'd need to do some research though to see if that is viable.

Since this isn't a Rock bug I'm going to close this as an issue. Please feel free to open it as a new Idea (bringing CSS inlining into core) or let us know if it's a feature you'd be interested in funding.

briankalwat commented 3 years ago

@jonedmiston so does this mean that the checkbox that states "CSS Inlining Enabled" that is displayed on all communication templates/system communications should be removed, since it doesn't actually do anything when you're using anything other than Mandrill? It being present in those locations gives the impression that it is Rock that is doing the inlining, rather than the communication transport.

image

jonedmiston commented 3 years ago

@briankalwat I don't think we should remove it as there may still be some organizations using Mandrill. We could add a clarification to the help text that says it's dependent upon support in the transport and update the documentation to specify the core transports that support it.

briankalwat commented 3 years ago

@jonedmiston It might be good to update this documentation, as well, as it makes it sound as if Mailgun is the official core-supported transport. Ideally, that "CSS Inlining Enabled" checkbox wouldn't be visible unless you had a transport enabled that supported it, but I trust whatever you think is best. Thanks!