fix image constraints

[jaredgalanis]

• images in markdown breakout of their container as described in adds markdown concept to show current state of markdown to html #335
• this sets a max width to constrain them

Co-Authored-By: Chris Manson mansona@users.noreply.github.com

[mansona]

@pichfl do you think this would have any negative effects? 🤔 the idea is to have a global rule because we can't add a specific class to images that are added to Markdown

[pichfl]

By setting only the max-width images may distort. max-width: 100%; height: auto; would change that but it might have unexpected side effects. We should rather fix anything that breaks though, I think having this as a base is reasonable.

[jaredgalanis]

By setting only the max-width images may distort. max-width: 100%; height: auto; would change that but it might have unexpected side effects. We should rather fix anything that breaks though, I think having this as a base is reasonable.

Thanks @pichfl! I'm sorry I need one point of clarification in follow up. Are you saying we should include height: auto or it is fine as is?

[mansona]

So @pichfl, I probably didn't explain the need for this change explicitly enough. When you say "We should rather fix anything that breaks though" what precisely do you mean? 🤔

The issue that we have is that we can't actually manually fix all images in some of the sites that use the ember-styleguide because we are making use of Markdown quite heavily. For this reason, we need to have sensible defaults defined in global styles that can apply without the need of specifying a class on the image.

For example, if you look at the example markdown page on master you will see that there is currently an image that is exploding off the side of the page: https://ember-styleguide.netlify.app/concepts/markdown/ (also you can see that the corresponding page is fixed in this PR: https://deploy-preview-341--ember-styleguide.netlify.app/concepts/markdown/ )

I'm happy for us to put in the height: auto rule if you think that would help this situation but I would be curious what you mean by "unexpected side effects" 🤔

[mansona]

Also I created two branches ember-learn/ember-website#666 and ember-learn/ember-website#665 that test with height: auto and without 🎉

Percy sees no difference between them but I guess that this could be because Percy uses Chrome? 🤔 Is there something that I should be looking for?

[ijlee2]

In case this CSS approach may help: https://github.com/ijlee2/ember-container-query/blob/main/tests/dummy/app/components/widgets/widget-3/tour-schedule/responsive-image/index.css

I don't think IE 11 supports object-fit, however. (grid either...)

[mansona]

@ijlee2 so this sort of solution isn't really available to us, we need to make sure that the default implementation (without using classes) works because of the nature of the way Markdown is being rendered in our infrastructure 👍 nice pointer though 😍 🎉

[mansona]

I'm going to go ahead with this change as is for now 👍 Because we ran the experiment with and without height: auto; and couldn't find any difference we think it's safe to leave it like this for the time being. If we find any evidence of distortions going forward we will look at what caused them and consider adding height: auto; as a bugfix at that stage 🎉