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 NavigationBar's title view constraints #1891

Merged

Conversation

laminesm
Copy link
Contributor

@laminesm laminesm commented Sep 11, 2023

Platforms Impacted

  • iOS
  • macOS

Description of changes

There has been an issue where the two line title view would not fit in our navigation bar (#1883). It turns out that the constraints of the title view were not set to fit the navigation bar (which is how Apple handles the native title view).
Changes:

  • Margins calculations were adjusted to give proper bottom insets to the title view
  • Lowered the priority of the height constraint on the TwoLineTitleView, so that autolayout can dynamically adjust it vertically in the nav bar
  • Added a bottom constraint to the titleView

Binary change

Total increase: 1,512 bytes
Total decrease: -488 bytes

File Before After Delta
Total 31,330,440 bytes 31,331,464 bytes ⚠️ 1,024 bytes
Full breakdown
File Before After Delta
NavigationBar.o 549,336 bytes 550,848 bytes ⚠️ 1,512 bytes
FocusRingView.o 818,960 bytes 818,888 bytes 🎉 -72 bytes
__.SYMDEF 4,826,776 bytes 4,826,360 bytes 🎉 -416 bytes

Verification

Tested fix on pro, pro max, and iPad devices on the fluent demo app.

Visual Verification
Before After
before_cell_5_pro_portrait after_cell_5_pro_portrait
before_cell_5_pro_landscape after_cell_5_pro_landscape
before_cell_10_pro_portrait after_cell_10_pro_portrait
before_cell_10_pro_landscape after_cell_10_pro_landscape
before_cell_11_pro_portrait after_cell_11_pro_portrait
before_cell_11_pro_landscape after_cell_11_pro_landscape

Pull request checklist

This PR has considered:

  • Light and Dark appearances
  • iOS supported versions (all major versions greater than or equal current target deployment version)
  • VoiceOver and Keyboard Accessibility
  • Internationalization and Right to Left layouts
  • Different resolutions (1x, 2x, 3x)
  • Size classes and window sizes (iPhone vs iPad, notched devices, multitasking, different window sizes, etc)
  • iPad Pointer interaction
  • SwiftUI consumption (validation or new demo scenarios needed)
  • Objective-C exposure (provide it only if needed)

@laminesm laminesm requested a review from a team as a code owner September 11, 2023 22:59
ios/FluentUI/Navigation/NavigationBar.swift Outdated Show resolved Hide resolved
@laminesm laminesm requested a review from huwilkes September 18, 2023 19:57
@laminesm laminesm merged commit bbf1959 into microsoft:main Sep 18, 2023
6 checks passed
@laminesm laminesm deleted the laminemale/fix-two-line-title-on-nav-bar branch September 18, 2023 22:10
laminesm added a commit to laminesm/fluentui-apple that referenced this pull request Sep 26, 2023
* Update titleView constraints to fit the nav bar

* Rewrite the logic

* Always update the title view contraints to fit

* Rename titleview constraints function

* Address comments

* Silence constraints warning
laminesm added a commit that referenced this pull request Sep 26, 2023
* Update titleView constraints to fit the nav bar

* Rewrite the logic

* Always update the title view contraints to fit

* Rename titleview constraints function

* Address comments

* Silence constraints warning
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