Zoom to fit

[simurai]

This PR adds a few improvements. Mainly

• "zoom to fit" mode
• permanent toolbar with controls
• Zoom stepper with zoom level
• transparent background button
• pixel rendering when an image is over 200%

Closes #39 + #37

[thomasjo]

Nice! 💖

[simurai]

Not sure how far this PR should go. There could still be more improvements or maybe make separate PRs for it? Some ideas:

• Showing the background color picker based on file type doesn't always make sense. Like when a png has no transparency, changing the background has no visual effect. So the "checkered" background could take up the whole space, not just the size of the image. But then it would still be nice to see the bounding box of an image, so maybe add a togglable border.
• Mouse support for zoom in/out.
• A slider for the zoom with percentages shown. That might also solve Display current zoom level #37.
• When zooming in on an image, would be nice to see the pixels instead of the image getting blurred. Adding image-rendering: pixelated; at a certain zoom level should fix it or a toggle in the toolbar.

/cc @thomaslindstrom saw that you contributed earlier, in case you have any feedback.

[thomaslindstrom]

Hi, @simurai – great work! This is in part why I've been annoyed with image-view recently. However, I'm not sure about whether or not “Zoom to fit” should be the default mode. Only because it's not what users currently might not expect image-view to do. Maybe the reason I'm unsure is because clicking the image to change modes feels unintuitive. So, maybe adding some controls to go with the “Checkered background color” toggles would help?

Showing the background color picker based on file type doesn't always make sense.

So with you on that one. This could be fixed by going through each pixel and figuring out if the alpha is 1 consistently, or if not, show the controls. Performance considerations, though.

Like when a png has no transparency, changing the background has no visual effect. So the "checkered" background could take up the whole space, not just the size of the image.

This. Great idea. Not entirely sure how to make the separation between “here is the image” and “this here is just background” clear enough. In “Zoom to fit” mode, the checkered background should be visible, too, imo. “Relying” on the current editor background feels off.

• Mouse support for zoom in/out.
• A slider for the zoom with percentages shown.
• When zooming in on an image, would be nice to see the pixels instead of the image getting blurred.

Yes!

I'd love to help improving image-view, so just give me a ping if you'd like me to join in. Again, great work!

[simurai]

@thomaslindstrom Thanks for feedback.

The idea with the "Zoom to fit" being the new default was more meant as a "quick view". You just want a quick glance if it's the right image, but not really need any details about the image. Keeping the UI as minimal as possible. Having a transparent background could be an issue, but the chance that it collides with the editor background, still "kinda" small? 😇

Then if you need more infos, you can click on the image and the top "toolbar" shows up with options to change background color, zoom in/out and other ideas like a slider, pixel/border toggle etc. To make it more obvious that you can click on an image, there could be an "show toolbar" button in the corner, like the current background picker.

Only because it's not what users currently might not expect image-view to do.

Yeah, it's always a bit risky changing the current default. A safer way would be to just add the ""Zoom to fit"" as a toggle and maybe in the settings you could choose to have it as the default.

[thomaslindstrom]

The idea with the "Zoom to fit" being the new default was more meant as a "quick view". You just want a quick glance if it's the right image, but not really need any details about the image.

I like the idea, but in more cases than not, this would result in one extra click for me. I think the safest bet would be merging the two into one view and then having a toggle button like the background color toggle.

Having a transparent background could be an issue, but the chance that it collides with the editor background, still "kinda" small?

Same here, I feel like keeping the checkered background is the safest bet. Seeing nothing that indicates “this is an image” could be confusing.

[simurai]

👍 ok, I'll try to change it from having 2 "modes" to just a toggle in the toolbar.

[simurai]

ok, now the toolbar is permanent and it starts off with white checkered and with actual size:

Personally I still prefer starting with a transparent and fit to zoom, but maybe that could be setting in the future.

[AbeEstrada]

I like the toolbar 👍

[simurai]

Also increased the zoom factor. And when an image is over 200%, the image-rendering changes to pixelated so you better can see the pixels instead of a blurry edge.

[simurai]

Something that could still be improved is when in "zoom to fit" mode and start zooming, it snaps to actual size first and zooms from there. The reason is because when using object-fit: contain; the proportions of an img might be different, so it needs to be reset first.

Maybe this can be fixed in another PR.

[mnquintana]

How about in the center of the "zoom stepper". Clicking it would go back to 100%

Yessssss that looks awesome 👍

[jerone]

Awesome work!

How about in the center of the "zoom stepper". Clicking it would go back to 100%:

Clicking the center might give users the suggestion that they can manually set a percentage value.
I would make the current percentage more readonly and add another button beside the Zoom to fit button as they both have a predefined zoom-level.

[winstliu]

Why not just make the percentage changeable instead of readonly?

[AbeEstrada]

Maybe click to 100% and double click to edit the percentage manually

[mnquintana]

I really like the suggestions everyone's making, but I wouldn't want to see this PR bogged down by feature creep - it's already a huge improvement over the current version. IMO it might be good to just make a few final small tweaks and 🚢 this, then refine it further in subsequent PRs to make things more manageable.

[simurai]

I guess we could do it like OS X that uses an input for the %.

Although it's not the default and you have to customize the toolbar to show those controls.

I wouldn't want to see this PR bogged down by feature creep

I can take a look how easy it is to change it, but otherwise leaving it for a next PR sounds good too.

[benogle]

leaving it for a next PR sounds good too

I agree with this 👍

[szhu]

Took me a while to figure out how to test this out.
Instructions for people reading this PR who can't wait to try it:

cd ~/.atom/packages
rm -rf image-view  # shouldn't exist but just in case
git clone https://github.com/atom/image-view.git image-view
git checkout sm-ui-improvements
apm install

[martinlindhe]

Awesome! Any updates on this?

[kankaristo]

Everyone seems to love this, merge please? 🙏

[EvanLovely]

Would love this too! I tried the above code and came across an error and realized it still needed to cd image-view after the clone; so the code snippet should be:

cd ~/.atom/packages
rm -rf image-view  # shouldn't exist but just in case
git clone https://github.com/atom/image-view.git image-view
cd image-view # missing line 
git checkout sm-ui-improvements
apm install

[maxbrunsfeld]

The code looks good to me, and as always, your UX is 👌 🌹 💸, @simurai. 🚢

[nathansobo]

@simurai I just tried this out, and it seems like zooming isn't working when the image is constrained horizontally. I saw it working in a GIF near the beginning of the PR, but check out what I'm seeing in my tests:

[nathansobo]

@simurai I pushed some changes that make zoom to fit work correctly when horizontally constraining things.

My trick was similar to my approach with the pane item views. I used position: absolute with a top left bottom and right of 0 to cause the image-container-cell to completely fill its containing flexbox item. Then I made the image 100% of the height and width of this containing element.

The browser seems to act counterintuitively when computing interactions with containers that are sized via flexbox, but the absolute trick is a way to bail out of flexbox and treat the parent as a more normal element. Maybe there's a more elegant approach, or maybe we should apply these styles more generally instead of predicating everything on zoom to fit. Another question: Does the image-container-cell node even need to exist? Either way, I'd like your sign-off before proceeding.

[simurai]

The "zoom to fit" should work again. Also removed image-container-cell which shouldn't be needed anymore.

Merge conflict got resolved by merging in master and reverting 2261f04. I think pathfinder isn't used anymore.

[nathansobo]

Awesome. Working correctly. Thanks @simurai!