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

Max Width Property on Email Logo #31

Closed
Samuel-Montoya opened this issue Sep 4, 2018 · 9 comments
Closed

Max Width Property on Email Logo #31

Samuel-Montoya opened this issue Sep 4, 2018 · 9 comments

Comments

@Samuel-Montoya
Copy link

There seems to be no max-width property on the email logo with the Salted Theme.
Is there any way this could be limited? On Outlook, some logos take up almost the entire screen if the image is too big.

This logo property is set in the product object when setting a theme.

Thanks!

@eladnava
Copy link
Owner

eladnava commented Sep 5, 2018

Hi @Samuel-Montoya,
Thanks for your suggestion.

Currently all themes come with a hard-coded max-height:

.email-logo {
max-height: 50px;
}

One option is to to resize the logo image you are providing to mailgen to the desired dimensions to make sure it doesn't occupy too much space.

The other option is to add an option to customize the max-height or add another option to customize the max-width of the <img class="email-logo" /> element. This would require modifying all 4 themes and mailgen core to inject the relevant pixel value into the HTML templates. Would you like to attempt this?

@Samuel-Montoya
Copy link
Author

I seem to have gotten a fix for my situation. For Outlook, the image element requires a width property to display correctly on the Outlook client.
<img src="<%- product.logo %>" width="200"/>

It would be wonderful if we were given the option to add or change this value, maybe in the product object?

product: {
        name,
        link,
	logo: LOGO_URL,
        width: 200
}

This would be especially nice so we don't have to manually resize pictures.
Just a thought. Thanks for your time.

@eladnava
Copy link
Owner

eladnava commented Sep 6, 2018

@Samuel-Montoya Sounds like a good plan. Are you comfortable with JS? Do you want to attempt adding this parameter in?

@Aptcoder
Copy link
Contributor

Aptcoder commented Oct 2, 2020

Hello @eladnava. Has this been implemented? Would like to help with it

@eladnava
Copy link
Owner

eladnava commented Oct 2, 2020

Hi @Aptcoder,
It has not. What do you say about adding an optional width integer attribute to the product object and injecting it into the themes' <img> tag if present?

@Aptcoder
Copy link
Contributor

Aptcoder commented Oct 2, 2020

I believe that is an appropriate solution. I would check for the property in mailgen core and set a default value if no value is given then use assign the attribute using ejs. I personally have not experienced the issue described as I do not have access to outlook mail but I believe this should be a fix

@eladnava
Copy link
Owner

eladnava commented Oct 2, 2020

Sounds good! Actually, sorry for the confusion, but it would be better if we modified the height and not the width, to maintain backward compatibility.

Currently, all themes set the logo max-height to 50px:

 .email-logo { 
   max-height: 50px; 
 } 

I would say the best approach would be to allow passing in a custom logoHeight (via product.logoHeight) and only if it was passed in, set it as a CSS property on the <img> tag. This would ensure backwards compatibility when people update mailgen.

@Aptcoder
Copy link
Contributor

Aptcoder commented Oct 3, 2020

Alright. I will make a PR to this effect

@eladnava
Copy link
Owner

eladnava commented Oct 3, 2020

Merged in #41.

@eladnava eladnava closed this as completed Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants