-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Bugfix FXIOS-11237-11238 Regression on settings change #24463
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, @lmarceau. I have tested this and it is working. The approach seems to be fine from my perspective because I have started the investigations on this issue but I have not found a fix, yet.
@lmarceau, tested, but the height bug is still there. |
Discussed with @PARAIPAN9 and there's another issue with the settings layout which we'll have a different ticket for. |
@lmarceau i think this should also be backported to 136. |
@Mergifyio backport release/v136 |
✅ Backports have been created
|
With boolean (cherry picked from commit 056b8ba)
…) (#24534) Bugfix FXIOS-11237-11238 Regression on settings change (#24463) With boolean (cherry picked from commit 056b8ba) Co-authored-by: lmarceau <[email protected]>
📜 Tickets
Ticket 1
Jira ticket
Github issue
Ticket 2
Jira ticket
Github issue
💡 Description
Changes in #24427 caused some regressions. Apparently reloading in
viewWillAppear
doesn't work, butviewDidAppear
works. One way to easily fix is by using a boolean. I am not a fan of this, but without putting too much time this is what I came up with. Let me know if you think of a better way. Tagging @dicarobinho since I saw you assigned yourselves one of the tickets, so maybe you have some ideas. Thank you!📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)