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

fix(style): sets max-width 100% for images in .content #382

Closed
wants to merge 1 commit into from
Closed

fix(style): sets max-width 100% for images in .content #382

wants to merge 1 commit into from

Conversation

edm00se
Copy link
Contributor

@edm00se edm00se commented May 9, 2018

An overly wide hero image will run out of the container and make the otherwise normally sized page
on mobile horizontally scroll-able.

refs: edm00se/awesome-board-games#5

fixes #381

An overly wide hero image will run out of the container and make the otherwise normally sized page
on mobile horizontally scroll-able. refs: edm00se/awesome-board-games#5

fixes #381
@edm00se
Copy link
Contributor Author

edm00se commented May 9, 2018

If this is desired more globally, this could easily be moved up to encompass all images, such as hero images. I noticed this on the home page, as it's where I was most likely to put a large image.

@edm00se edm00se changed the title fix(style): sets max-width 100% for images in content fix(style): sets max-width 100% for images in .content May 9, 2018
Copy link
Contributor

@ycmjason ycmjason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this is an important feature to be included in core. Maybe it is a good idea to include this globally as well. But let's see what @ulivz and @meteorlxy thinks.

@edm00se
Copy link
Contributor Author

edm00se commented May 9, 2018

😁

@ycmjason
Copy link
Contributor

ycmjason commented May 9, 2018

Well actually, we have this already at

The problem is probably because we have used <Content custom/> in Home.vue.

<Content custom/>

Not sure the reason behind this decision. We probably need to wait for @ulivz for advice. 😄

@edm00se
Copy link
Contributor Author

edm00se commented May 9, 2018

Curious, as the result wasn't observable in mine earlier today. Is that commit included in the 0.8.4 version?

Here's an example of it live, using [email protected].
edm00se/awesome-board-games#5

As an aside, it would be great if the git tags were to be re-introduced, as they seem to be quite a bit behind what's published to npm.

@ycmjason
Copy link
Contributor

ycmjason commented May 9, 2018

@edm00se
As I mentioned, this is because we have <Content custom/> which appends custom class to the container. So the style is not pulled through. Currently this style only apply to page contents.

@edm00se
Copy link
Contributor Author

edm00se commented May 9, 2018

Aha. Well, this ought to take care of it on the other side of things then.

Copy link
Contributor

@ycmjason ycmjason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be added to this instead even if actually needed

https://github.com/vuejs/vuepress/blob/master/lib/default-theme/styles/theme.styl#L70

@edm00se
Copy link
Contributor Author

edm00se commented May 9, 2018

Ah, I see, since it's already split logic. I'll close this PR and work up a new commit for the .content.custom section.

Thanks @ycmjason.

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

Successfully merging this pull request may close these issues.

Content Layout Bug: Image Runs Out Horizontally
2 participants