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

Let's set some sensible defaults for image styles #603

Closed
jenlampton opened this issue Jan 18, 2015 · 23 comments · Fixed by backdrop/backdrop#896
Closed

Let's set some sensible defaults for image styles #603

jenlampton opened this issue Jan 18, 2015 · 23 comments · Fixed by backdrop/backdrop#896

Comments

@jenlampton
Copy link
Member

The "Large" image style is not large. 480x480 is not large, that's more like medium

I recommend we adjust as follows:

  • Medium: scale & crop to 350x350
  • Large: scale to 960 wide (upscale no)

The width of the wider column in the two-column and two-column-flipped layouts is 615px. If we set the "large" image style to be 615x2 it (max-width: 100%) will look great on retina displays.

No upgrade path needed :)

@serundeputy
Copy link
Member

should we be changing the json files too?
image.style.large.json
etc.

@quicksketch
Copy link
Member

should we be changing the json files too?

We only need to change the JSON files that are included with Image module (/core/modules/image/config/image.style.x.json). That will affect new installs, but not existing sites (or those upgrading from Drupal 7).

I've been thinking more about the sizes that we should provide out of the box. Over in #378 (comment), we've been discussing the part of Picture module that we should port to Backdrop. In that thread, we talked about making images automatically provide a 2x size for high-DPI displays. That could make trying to accomodate for HiDPI displays now a bit premature.

I'm also worried about defining our sizes based on our current front-end theme (and for a particular layout). I'd feel better if we chose more arbitrary sizes that would work well (but perhaps not perfect) in a variety of themes and layouts. e.g medium at 480px and large at 960px

Thoughts?

@jenlampton
Copy link
Member Author

I updated the issue summary, I recommend we change Large to a more standard size, something like 960 wide. We may need to also add some CSS (to image module? to all themes?) so max-width for the large image style is set not to let these large images overrun our default front-end layouts though.

@klonos
Copy link
Member

klonos commented Jan 20, 2015

Do we take under account mobile and common resolutions when setting these defaults?

@jenlampton
Copy link
Member Author

How do you mean?

@klonos
Copy link
Member

klonos commented Jan 20, 2015

Like, "Large" should fit in the most common desktop/laptop screen/resolution, "Medium" in the most common tablet screen/resolution (landscape I guess) and the "Small" in the most common phone resolution.

Does that make sense?

@jenlampton
Copy link
Member Author

No, that doesn't make sense. The "Large" image style will be providing the home-page-slider-image on all three devices. (We're not changing image styles per device).

What we need to decide on is what we mean by "large". Then we set that. Then we make sure the "Large" image style resizes correctly (max-width: 100%) even on smaller devices.

@jenlampton
Copy link
Member Author

That said, those three sizes might just happen to coincidentally make good default sizes for the images ;)

@klonos
Copy link
Member

klonos commented Jan 21, 2015

We're not changing image styles per device

Ok, that's what I meant. Not per device though. Per device type.

@jenlampton
Copy link
Member Author

Right, we're not changing image styles per device type either. I think that's what breakpoint/picture module tried to do in D8. But all that hootenanny is yet to be proven in the real world, so it's too soon to do anything like that in core. Let's see what proves itself in contrib for responsive images and for now, we'll do the same thing as D7 as far as image styles go. Just with better default styles :)

Do you have an opinion as to what sizes we should use for large/medium?

@klonos
Copy link
Member

klonos commented Jan 21, 2015

Do you have an opinion as to what sizes we should use for large/medium?

Not really, was just asking to see what would qualify a size as "sensible". That's all.

@tjdjours
Copy link

Why not bypass the hardcoding of something that's relative (size: large, med, small) and name the sizes the numbers that they represent, eg '800' '450' etc.

Adding, identifying and referencing becomes a whole lot easier off the bat. And abstracting to a pre-defined relative scale requires everyone shuffle/redefine sizes according to that should they not match.. If a scalar pattern of 'small med large' or an absurdity like, 'Cmaj' fit a project, make us add it ourselves.

"Add medium-small. No, call it extra-medium-small.
Now add medium-medium-small"

@Lowell20
Copy link

+1 for (800x600 for example) image style naming scheme

@tjdjours
Copy link

(800x600 for example)

Yeah, what @Lowell20 said. With the y-dimension.

It is, arguably, the most sensible in practice.

@klonos
Copy link
Member

klonos commented Jan 21, 2015

I personally like it the way is now. Perhaps what we could do is auto-generate and append the dimensions to the style name on display (the selection drop-down in the "Manage display" page for example or the "Style name" column in the style list page).

@quicksketch
Copy link
Member

We can change the dimensions of our sizes for new installs, but if we rename the styles entirely, it'd be an API change for modules that expect the sizes thumbnail, medium, and large. I also like thumbnail, medium, large for the out-of-box presets because you're not allowed to change the machine names of your styles and you're also not allowed to delete default styles. So whatever we name it in the machine name, the user will be stuck with it.

@Lowell20
Copy link

explained well

@jenlampton
Copy link
Member Author

Why not bypass the hardcoding of something that's relative (size: large, med, small) and name the sizes the numbers that they represent, eg '800' '450' etc.

Because when you change your thumbnail from 100x100 to 90x90 its name still says 100x100, but that's now wrong :(

If we call it thumbnail you can make it whatever size you like without having to change the name. (Admittedly, this was more of a problem back before we had image style names, and changing the name of a style broke all the uses of that style on your site). I still think having human-understandable names out of the box is a huge advantage, but I agree that "Large" and "Medium" may not be good human readable names. I'd be happy to switch them to "Full" and "Teaser" to more closely line up with view modes (where they are actually used). That would make sense to everyone, and it would be clear that when you add new ones they don't need to fit in the same scale.

Anyway, I digress... If we want to debate changing the names, that's a 2.x issue (maybe someone create one?) and I want to make our image styles less dumb in 1.x :)

So back to the issue at hand - how about this:
Large: change to 800x600
Medium: change to 300x250

@tjdjours
Copy link

API change [..] out-of-box presets [.. ]you're also not allowed to delete default styles. So whatever we name it in the machine name, the user will be stuck with it.

Fair enough.

Because when you change your thumbnail from 100x100 to 90x90 its name still says 100x100, but that's now wrong :(

Good point. My approach is to add new styles rather than change them, but I recognize that's not always the case.

I'd be happy to switch them to "Full" and "Teaser" to more closely line up with view modes (where they are actually used)

'Thumbnail' is far more practical than 'extra small'. It's the arbitrarily set relative scale of S, M, L that is limiting and quickly becomes useless and in-the-way in projects that require a greater variety of sizes.

Anyway, I digress... If we want to debate changing the names, that's a 2.x issue (maybe someone create one?) and I want to make our image styles less dumb in 1.x

So back to the issue at hand - how about this:
Large: change to 800x600
Medium: change to 300x250

Yeah, keep the large low to help lead people to think about the quantity and range of sizes for their site.

@agreazel
Copy link

Starting work on this issue.

@quicksketch
Copy link
Member

I'm still up for including this in 1.1.0 if we can get it done in the next couple of days. Any takers?

@jenlampton
Copy link
Member Author

meeeee!

@quicksketch
Copy link
Member

Great, thanks! This was the last issue for Backdrop 1.1.0!

@quicksketch quicksketch reopened this May 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants