mmistakes / minimal-mistakes

:triangular_ruler: Jekyll theme for building a personal site, blog, project documentation, or portfolio.
https://mmistakes.github.io/minimal-mistakes/
MIT License
12.4k stars 25.56k forks source link

Header image aspect ratio depends on excerpt length #542

Closed DanielSank closed 8 years ago

DanielSank commented 8 years ago

Environment information

I expect the aspect ratio of a header image to be the same whether I use that image is used as a regular image or as an overlay image with an excerpt.

Steps to reproduce the behavior

See example post here.

The front matter looks like this


---
title: Modularity in LaTeX
header:
  image: header.png

---

and the image has a certain aspect ratio as shown:

normal

Now, if we change to overlay image with an excerpt,


---
title: Modularity in LaTeX
excerpt: blarg
header:
  overlay_image: header.png

---

then the image becomes much less tall, as shown here

thin

Interestingly, if we leave the excerpt line out, the auto-generated excerpt is much longer, and as a result the header image is taller.

To produce this error, I cloned the repo, adjusted the configuration, and wrote the post linked above.

mmistakes commented 8 years ago

The "hero" headers are fundamentally different, which is why you're observing differences in how they look.

header:
  image: image-filename.jpg

Default header.image uses a <img> element which has some CSS on it to scale it to fit the full width of the screen. The height scales proportionally to the width maintaining the image's aspect ration.

header:
  overlay_image: unsplash-image-1.jpg

While header.overlay_image does not use an <img> element and instead adds it as a background-image to div.page__hero--overlay. Since that image is added via inline CSS it doesn't know what the aspect ratio of the image. It simply applies a background image to the parent container.

If more content is added to it making the container larger the image will appear taller.

image__overlay_tall

If there's is less content it will appear shorter.

image__overlay_short

What you can do is add CSS to apply a height to that container. This could become problematic as the height will vary at different screen sizes, and will be especially hard to maintain if your images all have different aspect ratios.

I'm sure there is some sort of JavaScript magic you could apply, but that's out of scope of this theme.

DanielSank commented 8 years ago

What you can do is add CSS to apply a height to that container.

This leads to an obvious question: how does one apply CSS to specific elements, i.e. the height of the hero imagine on one specific post? I was trying to figure that out also in order to change the overlay title color, because some images make white titles hard to read. Unfortunately, I only figure out how to do this by editing the hard-coded value in _sass/_page.scss directly. Is there a way to edit the title color or image height on a per-post basis?

mmistakes commented 8 years ago

This hasn't been merged into master yet but there was an enhancement to allow custom classes.

The way this will work is you assign custom class names to a post or page's YAML Front Matter like so:

classes:
  - your-class-name

This will add your-class-name to the <body> element. Allowing you to specifically target the overlay with a selector like .your-class-name .page__hero--overlay, and then adding whatever CSS you need to style it.

I should be merging this into master shortly.

DanielSank commented 8 years ago

there's a discussion of custom classes in the docs, so I'm a little puzzled by you're saying that's not merged yet. Will go experiment. In any case:

  1. How does one add inline CSS to classes defined in the front matter? A trivial example in the docs would be helpful.
  2. Using custom classes seems a bit odd because we're then overriding CSS from _page.scss in our custom class. It's not obvious to me why the custom class properties would take priority. Is there some way to parametrize the stuff in _page.scss in a way that the parameters can be defined on a per-post basis? If this is not presently possible, I'd like to make it so, so any guidance you can give would be appreciated and might result in a pull request :)

I realize that in asking two questions here I might be going outside the bounds of a single issue. I can break up the discussion into smaller more focused ones if you prefer.

mmistakes commented 8 years ago

My bad. It was merged in. Too many themes too many issues too many PR, hard to keep it all straight :stuck_out_tongue:

You can't add inline CSS via Front Matter. You either have to add <style>your css</style> as part of the post/page's content or add those classes to main stylesheet.

The later is probably the better option. I'd create your own custom .scss, place it in _sass and then @import it into /assets/css/main.scss.

RE: 2. I get why that would be odd. Because of how the background images are assigned that's really the only option as you can't do it in _page.scss as that file doesn't read a post or page's YAML Front Matter.

Not saying it's an ideal option but it's certainly one of the more common approaches. Back when "art directed" posts were all the rage a few years ago that's how a lot of people did it eg: custom class overrides in the body of a post.

The custom classes take priority depending on the "cascade", hence the name CascadingStyleSheets. If you add a height or colors to .your-custom-class .whatever, it will take priority over .whatever because it of specificity. There are sites devoted to the topic that will explain it better than I can.

DanielSank commented 8 years ago

You can't add inline CSS via Front Matter. You either have to add as part of the post/page's content...

Ewwwwww

...or add those classes to main stylesheet. The later is probably the better option. I'd create your own custom .scss, place it in _sass and then @import it into /assets/css/main.scss.

Aha! I may be missing something really obvious, but even if I do this, how would I set attributes like the title color on a per-post basis? See next item.

RE: 2. I get why that would be odd. Because of how the background images are assigned that's really the only option as you can't do it in _page.scss as that file doesn't read a post or page's YAML Front Matter.

Yeah this gets at the real problem. Is there some way to parametrize the sass in a way that we can pick values of the parameters on a per-post content? If not, I'd really like to figure out a way to set that up.

Back when "art directed" posts were all the rage a few years ago that's how a lot of people did it eg: custom class overrides in the body of a post.

I see how that works, but it's not DRY and it messes up the separation of content and styling, so I'm interested in finding another way.

The custom classes take priority depending on the "cascade", hence the name CascadingStyleSheets.

I should have been more clear. I understand that CSS uses cascade to determine precedence. It's just not obvious whether custom classes are higher or lower in the cascade than the defaults because there's no specification about how custom classes propagate to the final document.

Thanks for your help so far.

DanielSank commented 8 years ago

To be clear, I am totally happy to do the legwork in setting up a way to parametrize CSS attributes on a per-post basis (assuming it's possible). Right now I'm trying to understand how the present system works and understand the boundaries of what's possible.

mmistakes commented 8 years ago

The present system isn't meant to be a full on custom CSS for every post/page slution. The theme isn't built for that nor will it ever be.

It's merely adding the ability to add custom classes to the body to use a simple JS hooks or to style an element here and there. If you're looking to inject custom CSS into every page to change title colors or anything else then the better solution would be to add your own custom Liquid to the `_layouts.

I have yet to see a DRY approach to this problem. It's possible if you're doing something simple like wanting an inverted style for every element to improve light on dark then sure. But if you're looking to do more than all bets are off unless you want to build a custom CSS file for every post/page.


And no, you can't currently do that with the way Jekyll reads assets. What you're looking to do is really not possible (well not easily at least). You'd essentially need to evaluate the Sass file(s) against every post/page to pull in YAML Front Matter to do what you want. And that likely wouldn't work as each post/page would just end up overwriting each other unless you're naming classes specifically.

My understanding of Jekyll is that isn't something done out of the box. You can ask around on talk.jekyllrb.com but without a custom plugin I think you are out of luck.


Two solutions I can think of, both of which involve you customizing layouts and includes to bend it to your will. Not something I'm interested in adding to the theme as it opens the doors to a million and one customizations as everyone will want something different. But if you're interested here's what you could do:

Option 1

Jam custom variables in _config.yml which are accessible from the site object. /assets/css/main.scss could then pull from those as it is processed by Jekyll. Any of the partials would not though since they don't contain YAML Front Matter and are treated by Jekyll as static assets.

A real simple example:

_config.yml:

custom-var1: green

/assets/css/main.scss:

.my-custom-var1 {
  color: site.custom-var1;
}

Option 2

Edit layouts and includes adding conditional statements where you want to apply customizations.

Example:

Say you want to change the post title's color using a value specified in it's YAML Front Matter.

post/page YAML:

title-color: green

And then modify the template:

<h1 class="page__title" {% if page.title-color %}style color: "{{ page.title-color }};"{% endif %}>

As you can see this has the potential to turn into a maintenance nightmare.

Again this is probably something that is super out of scope with this theme and dips more into "how to use Jekyll" land. I'd read out on talk.jekyllrb.com or Stackoverflow. Perhaps there are others who have a better solution.

DanielSank commented 8 years ago

The present system isn't meant to be a full on custom CSS for every post/page slution. The theme isn't built for that nor will it ever be.

Of course not. You must get a lot of people expecting your theme to solve all the world's problems...

If you're looking to inject custom CSS into every page to change title colors or anything else then the better solution would be to add your own custom Liquid to the `_layouts.

Yeah, that seems like a half-decent approach. Will use that as a stop-gap for now. I'll see if I can figure out a better way though.

Thanks for your help.

mmistakes commented 8 years ago

Of course not. You must get a lot of people expecting your theme to solve all the world's problems...

Naturally :wink: