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 the issue of detail view of SplitViewController when switching out-in the app #3118

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

Conversation

suhaibabsi-inst
Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst commented Feb 6, 2025

refs: MBL-18549
affects: Student, Teacher
release note: Fixed the issue where app doesn't refresh shown page properly after switching to another app

Issue Analysis

Upon testing on an isolated case, turned out that UISplitViewController always report collapse then expansion whenever app is put to background. For that, CoreSplitViewController in that case calls delegate's methods of:

func splitViewController(_ splitViewController: UISplitViewController, separateSecondaryFrom primaryViewController: UIViewController) upon expansion.

func splitViewController(_ splitViewController: UISplitViewController, collapseSecondary secondaryViewController: UIViewController, onto primaryViewController: UIViewController) upon collapsing.

Which causes the unnecessary disruption on SplitView, although nothing was actually changed from that perspective.

To get around this issue, a new property preBackgroundedSecondaryController to cache secondary view controller when the app is about to be backgrounded. And use that in the expansion delegate's method, instead of executing the DefaultViewProvider logic there, which assumes a real expansion is going on.

One note though, I think we need to rethink the protocol DefaultViewProvider as the main way to extract a secondary view controller in case of expansion. Apparently, it is not being utilized in course details except for home and assignments tabs.

Test Plan

See ticket's description.

Checklist

  • Follow-up e2e test ticket created
  • A11y checked
  • Tested on phone
  • Tested on tablet
  • Tested in dark mode
  • Tested in light mode
  • Approve from product

…t-in of the app

refs: MBL-18549
affects: Student, Teacher
release note: Fixed the issue where app doesn't refresh shown page properly after switching to another app
@inst-danger
Copy link
Contributor

inst-danger commented Feb 6, 2025

Teacher Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Feb 6, 2025

Student Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Feb 6, 2025

Release Note:

Fixed the issue where app doesn't refresh shown page properly after switching to another app

Affected Apps: Student, Teacher

MBL-18549

Coverage New % Master % Delta
Canvas iOS 91.52% 91.51% 0.01%

Generated by 🚫 dangerJS against dc0dc96

@suhaibabsi-inst suhaibabsi-inst marked this pull request as ready for review February 9, 2025 16:14
@suhaibabsi-inst suhaibabsi-inst self-assigned this Feb 11, 2025
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.

2 participants