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

[Lumen] LMN-1740: move geometry reader inside the component #2082

Merged

Conversation

hchen-skyscanner
Copy link
Contributor

@hchen-skyscanner hchen-skyscanner commented Oct 17, 2024

Recently, we found that the review gallery failed to show image in iOS 18 devices, here we try to resolve it

Before After
image image

Remember to include the following changes:

If you are curious about how we review, please read through the code review guidelines

@hchen-skyscanner hchen-skyscanner added wip minor Non breaking change labels Oct 17, 2024
@hchen-skyscanner hchen-skyscanner self-assigned this Oct 17, 2024
@hchen-skyscanner hchen-skyscanner marked this pull request as draft October 17, 2024 07:19
.padding([.leading, .top], .base)

ZStack(alignment: .bottom) {
GeometryReader { proxy in
Copy link
Contributor Author

@hchen-skyscanner hchen-skyscanner Oct 17, 2024

Choose a reason for hiding this comment

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

The only change we made here is move the GeometryReader from line 47 to 55.

After investigation, found that GeometryReader will return zero width during initialization (there's no such behavior in older iOS, we always get width = 375 during initialization).
So here we try to move the GeometryReader inside the ZStack, in order to defer the execution of GeometryReader to get the correct width.

Before After
image image

Place to log:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting investigation!!

@hchen-skyscanner hchen-skyscanner marked this pull request as ready for review October 17, 2024 08:39
@hchen-skyscanner hchen-skyscanner changed the title [WIP][Lumen] LMN-1740: move geometry reader inside the component [Lumen] LMN-1740: move geometry reader inside the component Oct 17, 2024
@frugoman frugoman merged commit bb036cb into main Oct 17, 2024
16 checks passed
@frugoman frugoman deleted the lumen/LMN-1740-Review-gallery-unable-to-show-image-in-iOS-18 branch October 17, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Non breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants