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

The Squash, Part Two (expand content images w/ JS) #2587

Closed
nfagerlund opened this issue Oct 21, 2019 · 6 comments
Closed

The Squash, Part Two (expand content images w/ JS) #2587

nfagerlund opened this issue Oct 21, 2019 · 6 comments
Assignees

Comments

@nfagerlund
Copy link
Contributor

In a bunch of styles, I added some CSS to shrink user-provided images (while preserving aspect ratio), so they don't overflow their container (or the viewport) too badly.

I think that's been mostly a win, but it's not a universal win; sometimes you want to see the whole image and don't mind if it blows out the page, and sometimes the image shouldn't be shrunk in the first place. (The latter is mostly just vertical comic strips like Questionable Content, which run afoul of the default behavior of "a portrait-orientation image should fit on one screen" -- if it's split into panels, it's actually a bunch of images anyway, so that doesn't make sense.)

We've been sharing some CSS hacks to work around that, but I think we can do better by adding a little JS-powered "expand/shrink" widget next to every image in user-provided content. That's the sort of fix that's impossible for users to provide in their own journals, but trivial to do sitewide.

Design problems to solve:

  • Definitely need to exclude userhead icons, poll bar graphs, and a variety of other things that are really more like UI elements than figures. This can also include things like stitched-together frame images in posts where someone went hogwild with decorative HTML. (At least post-content/comment-content areas are well-marked, so it's easy to not mess with the rest of the page.)
  • Ideally, only images that actually shrank should get the expand/shrink button. But how to detect that?
    • Most users don't provide height/width attributes when posting images, so info about the actual image dimensions isn't available at the time of DOMContentLoaded.
    • When the dimensions of the window change, that can shrink new images or unshrink ones that were shrunk.

So, a naïve implementation is pretty easy, but I want to think about this a bit more to see if we can get something that feels simple but avoids wacky side-effects.


Being selective about what we shrink is also an interesting idea (e.g. detecting really long skinny pics), but IDK if it's feasible yet and it wouldn't solve the entire issue anyway, so never mind that for now.

@nfagerlund
Copy link
Contributor Author

A good start is to compare HTMLImageElement.naturalWidth / naturalHeight with the standard width/height properties. (Can't do that until the load event, of course.) That leaves the question of how and where to handle window size changes.

@nfagerlund
Copy link
Contributor Author

If resize observer was a thing yet, I’d have it made in the shade, but no.

@nfagerlund
Copy link
Contributor Author

Hmm, also remember that new images can be added via cut tag or expand comment.

@nfagerlund
Copy link
Contributor Author

I thought of a more parsimonious option: Only target images that aren't inside links.

@supports (object-fit: contain) {

.entry-content img, .comment-content img {
  cursor: zoom-in;
  /* ...existing rules */
}
.entry-content img.expanded, .comment-content img.expanded {
  cursor: zoom-out;
  height: auto;
  width: auto;
  max-width: unset;
  max-height: unset;
  object-fit: unset;
}
.entry-content a img, .comment-content a img {
  cursor: unset;
}

}
$(document).on('click', '.entry-content img, .comment-content img', function(e) {
  if (! e.target.matches('a img') ) { // modern-ish, but anything that doesn't support it definitely doesn't support object-fit anyway, so it's fine.
    $(e.target).toggleClass('expanded');
  }
});

@nfagerlund
Copy link
Contributor Author

@nfagerlund
Copy link
Contributor Author

So now I need more test cases, and also a list of all the types of images within post bodies where we WOULDN'T want to offer a zoom click. Already excluding:

  • Any image inside a link. (Catches userheads, etc.) I'm assuming that for an image you'd want to expand, if it's in a link then 90% of the time it's linking out to a bigger version of itself, probably on a flickr or gphotos page (or DW media).
  • Poll results.

nfagerlund added a commit to nfagerlund/dw-free that referenced this issue Nov 27, 2019
Previously, we added CSS for limiting image sizes to most of the built-in
journal layouts. The goal was to avoid reading page blowouts (for mobile and
desktop alike) when people post large-ish images. It seems to be working as
expected 60-80% of the time, but it could be better. In particular, the max
height has a bad effect on vertical comic strips.

This commit extends and improves that image shrinking:

- Instead of managing image behavior per-theme, move it into a need_res-able
  component and include it on all journal pages.
- New JS behavior: let readers click images to expand/shrink them.
    - Use zoom-in/zoom-out cursors to show that you can click. (Cursors are
      desktop only, but expanding is far less relevant on mobile, so this makes
      sense.)
    - Images inside links are exempt. (1: they already have a click behavior. 2:
      if it's an image you'd want to expand, the link probably points to the
      full size version.)
    - Link exemption also catches UI things like userheads.
    - Poll bars get a special exemption.
- As before, this only affects images inside .comment-content and .entry-content.
nfagerlund added a commit to nfagerlund/dw-free that referenced this issue Nov 27, 2019
…youts

Now that this is a global behavior, it doesn't need to be duplicated in layouts.
nfagerlund added a commit to nfagerlund/dw-free that referenced this issue Nov 29, 2019
Previously, we added CSS that limits image sizes, to avoid reading page blowouts
when people post large images. This works as expected most of the time, but:

- It sometimes shrinks images it shouldn't, like vertical comic strips.
- Sometimes you just want to see the full size and don't mind a temporary blowout.

This commit handles those cases by adding a click-to-zoom behavior for images in
entry/comment contents. On desktop, a special cursor indicates when you can use
it. The new behavior only targets images that aren't inside links (since those
already have a click behavior), and has a special exemption for poll bar graphs.

This also moves the shrink CSS into a global fragment that gets pulled in with
need_res, so we don't need to maintain it in every layout.
nfagerlund added a commit to nfagerlund/dw-free that referenced this issue Nov 29, 2019
…youts

Now that this is a global behavior, it doesn't need to be duplicated in layouts.
zorkian pushed a commit that referenced this issue Nov 29, 2019
Previously, we added CSS that limits image sizes, to avoid reading page blowouts
when people post large images. This works as expected most of the time, but:

- It sometimes shrinks images it shouldn't, like vertical comic strips.
- Sometimes you just want to see the full size and don't mind a temporary blowout.

This commit handles those cases by adding a click-to-zoom behavior for images in
entry/comment contents. On desktop, a special cursor indicates when you can use
it. The new behavior only targets images that aren't inside links (since those
already have a click behavior), and has a special exemption for poll bar graphs.

This also moves the shrink CSS into a global fragment that gets pulled in with
need_res, so we don't need to maintain it in every layout.
zorkian pushed a commit that referenced this issue Nov 29, 2019
Now that this is a global behavior, it doesn't need to be duplicated in layouts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants