krille-chan / fluffychat

The cutest instant messenger in the [matrix]
https://fluffychat.im/
GNU Affero General Public License v3.0
907 stars 153 forks source link

Add settings to keep images aspect ratio #919

Closed BhasherBEL closed 3 months ago

BhasherBEL commented 4 months ago

This PR add a settings to force images to keep their initial aspect ratio in chat.

krille-chan commented 4 months ago

Thanks for the contribution but I don't think we should make this configurable as we would run into config creep if we do this with every change (unless we have a better configuration abstraction layer) how does the change look like and does it make sense to change this for all users?

M00NJ commented 4 months ago

I think it makes sense to retain the aspect ratio by default. When sending a screenshot often times important information is not visible in the chat and I worry the other person might not bother the open the image (since they don't know that I highlighted something at the top or bottom for example). This is also how all the messanger apps I've used so far handle it.

BhasherBEL commented 4 months ago

I agree that imo this should be the default behavior, as in many apps.

For now, the feature looks a bit ugly, as it simply force the picture to fit inside the box. It could instead resize the outside borders to also match the picture.

image

If you agree for both, I will implement them.

krille-chan commented 4 months ago

If you find a solution which looks good for all possible aspect ratios of pictures, sure. The current size should then just be a max width/height

BhasherBEL commented 4 months ago

The new commit should change what we have discussed. The sizes are constraints between 100-400 for width and 100-500 for height. For most cases (i.e. "normal sizes"), the width will be 400 and the height will be adaptive.

Here is how it looks like now: image

Here is how it handle extreme cases. image

krille-chan commented 4 months ago

Doesn't seem to work yet. When I send a photo which is very wide or very high but also very thin it looks like this:

Bildschirmfoto vom 2024-03-10 15-04-52

BhasherBEL commented 4 months ago

Did you use the last commit ? It's looks like the previous behavior where the box didn't scale. What happens if you use a standard picture (i.e. not extreme) ?

krille-chan commented 3 months ago

Did you use the last commit ? It's looks like the previous behavior where the box didn't scale. What happens if you use a standard picture (i.e. not extreme) ?

Yes this screenshot is from the newest version. The aspect ratio needs to work with all possible ratios, even with extreme ones

BhasherBEL commented 3 months ago

Did you use the last commit ? It's looks like the previous behavior where the box didn't scale. What happens if you use a standard picture (i.e. not extreme) ?

Yes this screenshot is from the newest version. The aspect ratio needs to work with all possible ratios, even with extreme ones

Definitely agree. But it's quite strange that it's still squared on your screenshot. It should never be squared again, except for real squared pictures.

krille-chan commented 3 months ago

Has now been implemented with a different approach. Anyway, thank you very much for the attempt