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

Cbowdoin/bottom sheet supports x axis constraints #1951

Merged

Conversation

cbowdoin
Copy link
Contributor

@cbowdoin cbowdoin commented Jan 24, 2024

Platforms Impacted

  • iOS
  • macOS

Description of changes

BottomSheetController.swift

Adds 2 new properties to bottom sheets: preferredWidth and anchoredEdge.
PreferredWidth allows for custom width specification for bottom sheets. The sheet will attempt to set the width to the size specified as long as the value is between the minimum and maximum width of the sheet. If the value is not within that range, we will revert back to the original behavior of using the max width.

To avoid conflicts with should always fill width these properties unset each other when one is set.
If should always fill width is set we will take this as the primary source of truth and defer behavior to this value being set.

AnchoredEdge allows the user to specify which edge we will constrain the sheet to. By default this remains keeping the sheet centered but when set users can move the sheet to be attached to the leading or trailing edge with a padding of 8.

BottomSheetDemoController.swift

  • Adds 2 new toggles that allow the user to test the preferredWidth and anchoredEdge.
    • preferredWidth will set the width to 400
    • the anchoredEdge toggle allows users to test attaching the sheet to the trailing edge. Disabling the toggle will
      recenter the sheet

Binary change

Total increase: 39,000 bytes
Total decrease: -3,816 bytes

File Before After Delta
Total 30,927,296 bytes 30,962,480 bytes ⚠️ 35,184 bytes
Full breakdown
File Before After Delta
BottomSheetController.o 490,440 bytes 521,072 bytes ⚠️ 30,632 bytes
__.SYMDEF 4,790,704 bytes 4,797,328 bytes ⚠️ 6,624 bytes
FocusRingView.o 815,024 bytes 816,768 bytes ⚠️ 1,744 bytes
BottomCommandingController.o 846,984 bytes 843,168 bytes 🎉 -3,816 bytes

Verification

Tested using the bottom sheet demo. Initialized the sheet with properties for the various scenarios.
Preferred Width

  • width declared to a value less than the minimum sheet size
  • width declared to a value greater than max sheet size
  • width declared to a value between min and maximum
  • width declared after the sheet is set to always fill width
  • width declared before the sheet is to always fill width

AnchoredEdge

  • centered, leading, and trailing all position the sheet in the correct spot in both LTR and RTL languages
Visual Verification
Before After
Screenshot 2024-01-24 at 2 52 08 PM Screenshot 2024-01-24 at 2 54 55 PM

Pull request checklist

This PR has considered:

  • Internationalization and Right to Left layouts
  • Size classes and window sizes (iPhone vs iPad, notched devices, multitasking, different window sizes, etc)
  • Objective-C exposure (provide it only if needed)
Microsoft Reviewers: Open in CodeFlow

@cbowdoin cbowdoin marked this pull request as ready for review January 24, 2024 23:14
@cbowdoin cbowdoin requested a review from a team as a code owner January 24, 2024 23:14
Copy link
Contributor

@laminesm laminesm left a comment

Choose a reason for hiding this comment

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

Is this change meant to be iPad only?

@cbowdoin
Copy link
Contributor Author

Is this change meant to be iPad only?

No. The most likely scenarios where this is useful is on iPad but I see no reason to limit consumers to use it on iPad. For example, I could see a scenario where you would want a bottom sheet that is presented on iPhone in landscape in a non centered view.
Screenshot 2024-01-25 at 9 57 35 AM

@cbowdoin cbowdoin force-pushed the cbowdoin/BottomSheetSupportsXAxisConstraints branch from 086b419 to bf218a5 Compare January 25, 2024 18:24
@cbowdoin cbowdoin closed this Feb 16, 2024
@cbowdoin cbowdoin reopened this Feb 16, 2024
@cbowdoin cbowdoin enabled auto-merge (squash) February 20, 2024 17:37
@cbowdoin cbowdoin merged commit 7a1bc97 into microsoft:main Feb 20, 2024
7 checks passed
@mischreiber mischreiber mentioned this pull request Mar 6, 2024
12 tasks
@sophialee0416 sophialee0416 added the New API This PR introduces new API label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New API This PR introduces new API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants