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

Downsize posts with a tall photo #127

Closed
jwsp1 opened this issue Mar 18, 2019 · 20 comments
Closed

Downsize posts with a tall photo #127

jwsp1 opened this issue Mar 18, 2019 · 20 comments

Comments

@jwsp1
Copy link

@jwsp1 jwsp1 commented Mar 18, 2019

Posts that contain tall photos (e.g. screenshots) are also very long, and the image can't usually be viewed in its entirety without clicking on it.

You could optionally post images in a square format (with a border on the sides, similar to Instagram) and make them enlarge with a click.

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Mar 18, 2019

Thanks for reporting @jwsp1 !

@lifenautjoe lifenautjoe added this to To do in Openbook Beta via automation Mar 18, 2019
@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Mar 27, 2019

Limiting the image to a certain max height and showing an expand image button might be the way to go.

image

Not ugly as shown in this image, just a simple block using the same theme-dependent color that the OBAlert uses and highlighted with the theme primary accent color.

@jwsp1

This comment has been minimized.

Copy link
Author

@jwsp1 jwsp1 commented Mar 27, 2019

Limiting the image to a certain max height and showing an expand image button might be the way to go.

[image]

Not ugly as shown in this image, just a simple block using the same theme-dependent color that the OBAlert uses and highlighted with the theme primary accent color.

Nice idea but I'd rather prefer the whole image at a first glance... (Maybe it's just by bad taste though ;) )
Have you considered actual downsizing by putting the image in a square format? Expand to full size by click.
Unbenannt

@Komposten

This comment has been minimized.

Copy link
Member

@Komposten Komposten commented Mar 30, 2019

Looking into the down-scaling option at the moment.
On top of down-scaling large images (based on the width of the device), I also changed it so it doesn't upscale small images (unless they are smaller than 10% of the screen width).

Attaching a couple of images to show what it looks like for a few situations:
image

(@lifenautjoe, what's your/your team's opinion on this approach.)

Note: This might look a bit weird for really, really tall images (like some comics which can be 10 times as tall as wide) as they will become extremely thin.

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Mar 30, 2019

@Komposten

This comment has been minimized.

Copy link
Member

@Komposten Komposten commented Mar 30, 2019

When you say "max height [...] showing some indication [...] to expand", do you mean "show only part of the image, but have an indicator to show there's more" (akin to the "Expand Post" button in your screenshot above, albeit a bit more discrete and Openbook-y)?

If that's the case, it does have the problem that most portrait photos would trigger this, which could be annoying if many such photos appear in ones feed (can't just see them directly without having to tap-view-close).

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Mar 30, 2019

@jwsp1

This comment has been minimized.

Copy link
Author

@jwsp1 jwsp1 commented Mar 30, 2019

Tbh, I don't want to scroll through my feed and click a thousand of times to expand all tall photos. I want to see the content with at one glance.
If that's really the plan, I'd be happy to see an option to disable this "improvement". Personal opinion - maybe I have a bad taste 😜

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Mar 30, 2019

@Komposten

This comment has been minimized.

Copy link
Member

@Komposten Komposten commented Mar 30, 2019

I'll give it a shot and see how it turns out. 🙂

I still think that images that are smaller than the screen width should remain small (unless they are too small), and not be up-scaled to fit the width. Both goes for pictures in the timeline and when they are tapped on.

The image in the posts titled "Very tiny" in my screenshot above looks like this when upscaled:
image

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Mar 30, 2019

@Komposten

This comment has been minimized.

Copy link
Member

@Komposten Komposten commented Mar 30, 2019

Can someone who's more artistically gifted than I do a mock-up of what it should look like? (Need an icon to use, and probably some visual indication that the bottom of the image is cut off.)

Also, should the expand button expand the image in situ or open the zoomable photo modal?

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Mar 30, 2019

@Komposten

This comment has been minimized.

Copy link
Member

@Komposten Komposten commented Apr 2, 2019

@lifenautjoe I know you've been really busy the last few days with trying to get the timeline optimisations working, so don't worry about this until you have time. Still, just bumping it in case you forgot about it. 😉

(Also, you can probably assign me this one since I've started working on it.)

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Apr 13, 2019

Without expand
wp-expand-iphone

With expand
expand-iphone

Basically the image is set as a square (screen.width * screen.width) with fit: BoxFit.cover property with a button on the right which indicates it can be expanded.

When tapped, preserves the same current behavior of opening the img in the image preview modal.

@Komposten

This comment has been minimized.

Copy link
Member

@Komposten Komposten commented Apr 13, 2019

That's almost exactly what I have (although I'm using a different aspect ratio limit). I think I basically just need to add the background and change the icon of the button. 👍

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Apr 13, 2019

Awesome! 👯‍♂️

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Apr 13, 2019

Question, is the max height configurable? perhaps we can make it a ratio of the height of the screen. If a post is > 3/4 of the screen height, display like this 🤔 .

@Komposten

This comment has been minimized.

Copy link
Member

@Komposten Komposten commented Apr 13, 2019

This is the current code: imageHeight < screenHeight*.75 ? imageHeight : screenHeight*.75
So yep, I can make it whatever we want.

Edit: Note that screenHeight is the actual height of the screen, including the action bar and the tab bar at the bottom. So with 75% the image still covers most of the visible space between those two on my screen.

@lifenautjoe

This comment has been minimized.

Copy link
Member

@lifenautjoe lifenautjoe commented Apr 13, 2019

Alright, seems reasonable. I'll test it out once you give green light 🚦 .

@lifenautjoe lifenautjoe moved this from To do to Waiting for mobile deploy in Openbook Beta Apr 18, 2019
Openbook Beta automation moved this from Waiting for mobile deploy to Done Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Openbook Beta
  
Done
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.