Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Altered froala image plugin to allow imageOutputSize to function on i… #7569

Merged

Conversation

@robopossum
Copy link

robopossum commented May 29, 2019

…mage slots to fix displaying in Outlook

Please be sure you are submitting this against the staging branch.

Q A
Bug fix? Y
New feature? N
Automated tests included? N
Related user documentation PR URL /
Related developer documentation PR URL /
Issues addressed (#s or URLs) #2791
BC breaks? N
Deprecations? N

Description:

#6845 was merged to fix issue with images displaying incorrectly in Outlook, but only fixed it for Text slots in the email builder. This PR applies the same change to Image slots.

It also contains changes to the froala image plugin, as in its current state the imageOutputSize option has no effect on editors instantiated on img tags.

Steps to reproduce the bug:

  1. Create an email.
  2. Add an Image slot and select an image.
  3. Save the email.
  4. Preview the email.
  5. Inspect the HTML source and the image doesn't have the width and height attributes.

Steps to test this PR:

  1. Create an email
  2. Add an Image slot and select an image.
  3. Save the email.
  4. Preview the email.
  5. Inspect the HTML source and the image has the width and height attributes set.
…mage slots to fix displaying in Outlook
@npracht npracht added this to the 2.16.0 milestone Jun 25, 2019
@npracht npracht added this to Ready to Test (first time) in Mautic 2 Aug 15, 2019
@npracht npracht modified the milestone: 2.16.0 Jan 23, 2020
@maxluijben

This comment has been minimized.

Copy link

maxluijben commented Mar 9, 2020

Hi there, i'm looking much forward to this feature. Any updates about when it will be released and integrated into Mautic? As far as i understood it would be integrated in Mautic 2.16.0 but so far i couldn't notice this functionality.

I'm new to Github so maybe i'm asking something i could've figured out by myself. I apologize in that case.

@RCheesley

This comment has been minimized.

Copy link
Member

RCheesley commented Mar 9, 2020

@maxluijben I was literally just looking into tagging this for 2.16.1. We would need two successful tests to merge it, would you like to help us with testing the issue if it's something you're experiencing?

@RCheesley RCheesley added this to the 2.16.1 milestone Mar 9, 2020
@npracht npracht added this to Needs a second test/review in Mautic 2 Mar 10, 2020
@npracht npracht moved this from Needs a second test/review to Ready to test in Mautic 2 Mar 10, 2020
@dennisameling dennisameling added the L1 label Mar 18, 2020
Copy link
Member

RCheesley left a comment

Tested and could reproduce the lack of width/height before patch, and inclusion with the patch. 👍 thank you for the patch and sorry for the delay in getting it processed!

@RCheesley

This comment has been minimized.

Copy link
Member

RCheesley commented Mar 18, 2020

@maxluijben if you'd like to test this, simply go to mautibox.com, then in the dropdown type the PR number (7569) and it will spin up a Mautibox instance with this PR applied.

Create an email, add an image slot, upload your image, save, preview, inspect the image and check for the width/height.

If all are OK, please report back here as a successful test and we can get this merged :)

@RCheesley RCheesley moved this from Ready to test to Needs a second test/review in Mautic 2 Mar 18, 2020
Copy link
Member

dennisameling left a comment

Before:

image

After (using Mautibox):

image

Width + height properties are added correctly. Approving + merging this PR for 2.16.1 👍

@dennisameling dennisameling merged commit 649b4ec into mautic:staging Mar 19, 2020
2 checks passed
2 checks passed
Scrutinizer Analysis: No new issues – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Mautic 2 automation moved this from Needs a second test/review to Merged Mar 19, 2020
@mautibot

This comment has been minimized.

Copy link

mautibot commented Mar 20, 2020

This pull request has been mentioned on Mautic Community Forums. There might be relevant details there:

https://forum.mautic.org/t/announcing-mautic-2-16-1-beta/13438/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Mautic 2
  
Merged
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.