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

Fix VectorHostingController infinite loop #6381

Merged
merged 3 commits into from
Jul 7, 2022

Conversation

ismailgulek
Copy link
Contributor

We've introduced an infinite loop on VectorHostingController when we tried to solve the safe area insets issue with method swizzling, by #6374. This PR removes the method swizzling and fixes the infinite loop.

@ismailgulek ismailgulek requested review from a team and SBiOSoftWhare and removed request for a team July 6, 2022 10:23
@sonarcloud
Copy link

sonarcloud bot commented Jul 6, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov-commenter
Copy link

Codecov Report

Merging #6381 (fc01010) into develop (2babca6) will decrease coverage by 0.35%.
The diff coverage is 36.84%.

@@             Coverage Diff             @@
##           develop    #6381      +/-   ##
===========================================
- Coverage     6.54%    6.18%   -0.36%     
===========================================
  Files         1444     1444              
  Lines       155533   155474      -59     
  Branches     62507    62481      -26     
===========================================
- Hits         10179     9616     -563     
- Misses      144948   145455     +507     
+ Partials       406      403       -3     
Impacted Files Coverage Δ
...linePoll/Coordinator/TimelinePollCoordinator.swift 0.00% <0.00%> (ø)
...dules/Common/SwiftUI/VectorHostingController.swift 71.23% <41.17%> (-0.65%) ⬇️
Riot/Modules/LaunchLoading/LaunchLoadingView.swift 0.00% <0.00%> (-100.00%) ⬇️
...es/LaunchLoading/LoadingAnimation/Timeline_1.swift 0.00% <0.00%> (-100.00%) ⬇️
...s/LaunchLoading/LoadingAnimation/ElementView.swift 0.00% <0.00%> (-93.03%) ⬇️
Config/CommonConfiguration.swift 85.71% <0.00%> (-5.20%) ⬇️
Riot/Managers/Call/CallPresenter.swift 1.08% <0.00%> (-5.14%) ⬇️
...rs/EncryptionKeyManager/EncryptionKeyManager.swift 35.41% <0.00%> (-4.17%) ⬇️
Riot/Modules/Application/LegacyAppDelegate.m 12.88% <0.00%> (-2.52%) ⬇️
...dules/MatrixKit/Models/Account/MXKAccountManager.m 17.19% <0.00%> (-1.11%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2babca6...fc01010. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Jul 6, 2022

📱 Scan the QR code below to install the build for this PR.
🔒 This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.

QR code

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/i29LUb

@Johennes Johennes self-requested a review July 6, 2022 14:24
Copy link
Contributor

@Johennes Johennes left a comment

Choose a reason for hiding this comment

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

Tested this with various polls and it seems fine to me. Glad we don't have to swizzle anymore. Thanks for fixing this!

@SBiOSoftWhare
Copy link
Contributor

Is it something we should do at the UIHostingController subclass level or handle specific cases with the .ignoresSafeArea() modifier in SwiftUI views?

@Johennes
Copy link
Contributor

Johennes commented Jul 6, 2022

Is it something we should do at the UIHostingController subclass level or handle specific cases with the .ignoresSafeArea() modifier in SwiftUI views?

.ignoresSafeArea() didn't fix the layout issue for polls in my testing, sadly.

@ismailgulek ismailgulek merged commit e9257a0 into develop Jul 7, 2022
@ismailgulek ismailgulek deleted the ismail/fix_vectorhostingcontroller_loop branch July 7, 2022 07:50
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.

4 participants