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 iOS 18 iPad issues #2983

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

Conversation

vargaat
Copy link
Contributor

@vargaat vargaat commented Nov 22, 2024

refs: MBL-18079
affects: Student, Teacher, Parent
release note: none

test plan:

  • Test all three apps on iPads/iPhones both iOS 17/18 (use the checklist at the bottom of the PR).

What was fixed?

  • Opening the profile menu sometimes didn't dim the screen and closing the menu by tapping outside of it was impossible. This was caused by an animation not being run in SideMenuTransitioning. I changed the animation from the alongsideTransition since that should only be used to animations outside of the transition's transitionView.
  • The tab change has a new transition animation that shrinks and enlarges the tab's content. This looks nice on views with white navigation bars or without any navigation bars but look weird when the white background flashes around a dark navigation bar for a split second upon tab change. I changed the transition to a simple quick fade animation.
  • On iOS 18 iPads when there was enough space for the app being in split mode the tab bar got elevated and placed inside the navigation bar. This looked strange on the dashboard because the navigation bar was vertically very large with its bottom area only holding the logo image at its center. I made the rendering of this logo image dynamic and getting removed in case of an elevated tab bar.
  • The elevated tab bar's text color was black, I modified it to respect the brand's color as it does with a regular tab bar.

Known issues

  • The elevated tab bar's text color is always white in dark mode despite we set the brand's color that works in light mode.
  • When the a tab's content is displayed for the first time its detail view resizes on screen due to the elevated tab bar. This causes a flicker.

Screenshots

BeforeAfter

Checklist

  • Tested on iPhone (iOS 17)
  • Tested on iPhone (iOS 18)
  • Tested on iPad (iOS 17)
  • Tested on iPad (iOS 18)

- Add custom tab change animation not to use the default one on iOS 18.
- Hide dashboard nav bar icon when elevated tab bar is used.
refs: MBL-18079
affects: Student, Teacher, Parent
release note: none

test plan:
- Test all three apps on iPads/iPhones both iOS 17/18.
@inst-danger
Copy link
Contributor

inst-danger commented Nov 22, 2024

Parent Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Nov 22, 2024

Teacher Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Nov 22, 2024

Student Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Nov 22, 2024

Warnings
⚠️ This pull request will not generate a release note.
⚠️ One or more files are below the minimum test coverage 50%

Affected Apps: Student, Teacher, Parent

MBL-18079

Coverage New % Master % Delta
Canvas iOS 90.15% 90.16% -0.01%
Core/Core/Planner/CalendarMain/ViewModel/PlannerViewModel.swift 0% 0% 0%
Core/Core/Planner/CalendarMain/Model/CalendarWeek.swift 0% 0% 0%
Core/Core/Planner/CalendarMain/View/Calendar/CalendarCardInteractionState.swift 0% 0% 0%
Core/Core/Planner/CalendarMain/Model/PlannablesInteractor.swift 0% 0% 0%
Core/Core/Planner/CalendarMain/View/Calendar/ViewPreferences.swift 0% 0% 0%
Core/Core/Planner/CalendarMain/Model/CalendarDay.swift 0% 0% 0%
Core/Core/Planner/CalendarMain/Model/CalendarMonth.swift 0% 0% 0%
Core/Core/Planner/CalendarMain/View/Calendar/CalendarUtils.swift 0% 0% 0%

Generated by 🚫 dangerJS against 864e5f9

@vargaat vargaat self-assigned this Nov 22, 2024
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