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

Changed item container max-width from 70% to 37% to render attachments at a smaller size #55

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

sabogalc
Copy link

Below are screenshot comparisons between SMServer, macOS, the previous attachment loading in MyMessage, and my changed version. This might not be perfect/final so please let me know what you think of it.
SMServer
Screen Shot 2021-06-29 at 6 02 08 PM
Old MyMessage
New MyMessage
SMServer 2
Screen Shot 2021-06-29 at 6 02 34 PM
Old MyMessage 2
New MyMessage 2

@sabogalc
Copy link
Author

sabogalc commented Jun 29, 2021

One unintended/unfortunate side effect of this change is that attachments kind of flash and resize strangely when they load in now. I'm not sure how to go about fixing that.

React.Redux.App.-.Brave.2021-06-29.18-26-19.mp4

Edit - As far as I can see, this pull request fixed the flashing issue in that the flash is now the same color as the background, so it is not visible.

@sabogalc
Copy link
Author

sabogalc commented Aug 14, 2021

I know 37 seems like a super random number, but I tried 35 and it was too small, and I tried 40 and it was too big, so from there I went on to try everything in between, and 37 ended up being the Goldilocks number.

As noted in my original feature request, I initially tested this issue by seeing what percentage of horizontal space a specific vertical image took up on the screen. macOS was 36.18%, and iOS was 36.36%, for an average of 36.27%. Between these three numbers, it would make sense that in my internal testing, 35 was too little, and while the numbers to round down to 36, 37 looked better to my naked eye. Maybe we could also just implement a decimal percentage for max-width, but I had totally forgotten to check for that when I initially made this change. I just kind of eyeballed it, and by chance, my random number selection ended up being fairly close to what I initially measured and observed.

@sabogalc
Copy link
Author

Recently, I got a Framework laptop, and it has a 3:2 display rather than the usual 16:9. This is an issue I had with a tapback placement not quite lining up with the URL preview. I don't know if this is due to me running a custom build that has this 37% change, or if it's the 3:2 display, or something else.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant