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

Port the media layout algorithm from the Android app #992

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

grishka
Copy link
Member

@grishka grishka commented Mar 27, 2023

As discussed, I tried to port the media layout algorithm from the Android app. Android sources: the layout algorithm itself and the custom ViewGroup that actually puts the views on the screen. This new layout algorithm works with up to 10 media attachments.

IMG_0876

I'm not familiar with Swift and UIKit, and definitely not with ReactiveSwift that permeates the entire codebase of the app. I might have done something stupid.

This PR isn't quite ready for merging. There's something inside MediaView that doesn't respect its frame and won't crop vertically, resulting in overlapping views:
IMG_0874
IMG_0875

There's also a bug I don't know how to fix because I don't know where to hook into either the app or UIKit to re-run the layout with new maxSize when the view size changes. Android would automatically measure and then lay out the view subtree when the dimensions of its parent ViewGroup change, but iOS apparently doesn't do that? To reproduce, open an attachment, rotate the device to landscape orientation, then close the attachment.

@CLAassistant
Copy link

CLAassistant commented Mar 27, 2023

CLA assistant check
All committers have signed the CLA.

@grishka
Copy link
Member Author

grishka commented Mar 27, 2023

Also, teeny-tiny gifs. I'm definitely missing some crucial piece of understanding of how UIKit views work.

IMG_0877

@j-f1
Copy link
Contributor

j-f1 commented Mar 27, 2023

I think you need to call super.layoutSubviews() in your layoutSubviews otherwise the subviews won’t layout their own subviews.

@grishka
Copy link
Member Author

grishka commented Mar 27, 2023

Nope, it made no difference =/

MediaLayoutResult.Tile(colSpan: 1, rowSpan: 1, startCol: 2, startRow: 1)
])
} else {
// One on the left, three smaller ones on the right
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean 4 attachments will never be rendered as a 2x2 grid?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's always 1 bigger one and 3 smaller ones

@grishka
Copy link
Member Author

grishka commented Mar 30, 2023

I limited the aspect ratio to 9:20 to avoid weird skinny layouts with images of extreme aspect ratios. I chose 9:20 specifically so that vertical screenshots from modern phones don't end up getting cropped. Modern "bezel-less" phones have 9:18-ish screens.

Simulator Screen Shot - iPhone 14 Pro - 2023-03-30 at 18 56 43

@grishka
Copy link
Member Author

grishka commented Apr 2, 2023

Another interesting observation — if a post has a broken layout, but I then tap it to open separately, that screen has a correct layout.

Might it be that images loading asynchronously affect the layout inside MediaView somehow?

@grishka
Copy link
Member Author

grishka commented Mar 30, 2024

I decided to take another look at it and remembered that the layout inspector exists. So something is adding these constraints to the UIImageView, making the layout break when I set frames manually. Now it's only a matter of finding what it is.
Снимок экрана 2024-03-30 в 12 50 12

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.

3 participants