-
Notifications
You must be signed in to change notification settings - Fork 167
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
Bottom sheet collapsed height resolver API #1207
Conversation
test on iPad as well please? attach video of the usuage? |
a79e57e
to
4a26070
Compare
Tested on iPad + attached vids of the demo on both iPhone and iPad. |
4a26070
to
9194eb0
Compare
* make sure macOS Avatar respect custom color (#1196) -new private var to bookkeep when the colors are meant to be custom vs default -remove unnecessary initialsTextField background coloring. It should be just handled by the initailsView itself. -add a test case in TestAvatarController to have avatar hosted by NSPopover * Fix Voice Over accessibility for title in TableViewHeaderFooterView w… (#1197) ISSUE When Voice Over accessibility is enabled, the TableViewHeaderFooterView title has a wrong selection when presented in a popover on iPad. The title in this header/footer is a UITextView and in the existing implementation we override the default accessibility behavior by making the UITextView an accessibility element. This implementation leads to a wrong text selection for title in popover on iPad. FIX The fix is to remove the accessibility element assignment from UITextView and keep the accessibility trait insertion as .header when TableViewHeaderFooterView is used as header and remove the accessibility trait insertion when the view is used as footer. This implementation already exists but it was followed by a 'Bug in iOS - ...' implementation where accessibility element for UITextView was set up to 'false' and then back to 'true'. That implementation broke the VO selection for multi-line of text title for both header and footer scenarios. Removing that logic fixed the issue properly. * introduced a Medium Fluent Button Size * Fixed swiftlint issues * Add Objective C demo for PopupMenu (#1198) * Add Objective C demo for PopupMenu * Move setup to loadVIew * Set stack alignment to top to resize button * Remove comments * Add selected image for Toronto so it isn't a blue square * Fix indentation * Fix whitespace * Move ObjC to middle of name * Merging `ControlTokenSet` to main branch (#1206) * Merging ControlTokenSet from fluent2-tokens * Delete accidental file merge * Restore lost code in separator merge (#1212) * Revert "PillButtonBar: Fixing warnings for iOS 15 target deployment version. (#1165)" (#1213) This reverts commit 8d26fe7. * Align Notification Border with Figma (#1203) * Align Notification with Figma * Revert naming and token change * Align Notification with Figma * Revert naming and token change * Rebase onto main * Delete dupe edge border * Revert removing default bool * Implement functionality to show a custom view within a CommandBarItem (#1210) Implement functionality to show a custom view within a CommandBarItem * cherry-pick 45c022d (#1217) Co-authored-by: Sophia Lee <sophia.lee0416@gmail.com> * sanitization * sanitization * Fix tokens, text width, and showDefaultDismissActionButton demo (#1215) * Remove all usage of UIScreen.main (#1216) UIScreen.main is deprecated in iOS 16 (https://developer.apple.com/documentation/uikit/uiscreen/1617815-main). Most of the usage of UIScreen main is legacy code that tries to figure out device pixel. But, in reality, in order to avoid anti-aliasing we should be just making sure we draw on integer points. If we are trying to draw border, let's just draw on half point for now. * update demo windowscene * demo app only: tableviewcell accessory adhering to 1pt borderwidth and 2pt corner radius which are closer to fluent 2 design tokens * remove usage of UIScreen.middleOrigin * remove usage of roundDownToDevicePixels * remove usuage of deviceLineHeight. just use UIFont's lineheight * delete UIFont extension. * remove usage of roundToDevicePixels * remove UIScreen+extension * Separator height to be half a point * remove UITableViewCell+extension * remove uiscreen.main.bounds usage * remove file reference that doesn't exist * use guard let to figure out the window for drawercontroller * Update GradientInfo to specify the gradient is linear (#1218) * Update documentation to be explicit about linear * Add Linear to name of GradientInfo * Remove UIEdgeInsets from SegmentedPillButton (#1221) * Remove UIEdgeInsets from SegmentedPillButton * Add TODO * Move `fluentTheme` from window to view (#1219) * Move `fluentTheme` from window to view * Add logic to only refresh if view is a descendent of the newly themed view * allow colorful image in notification toast Currently we always render the toast image as template. If the client team pass in image that is set "renderAsOriginal" respect that mode. * Added a minButtonHeight to account for the fact that the button height currently grows to fit the image size within it * add new module map in the fluentuilib and add copy phase (#1231) use FluentUI as a clang module in objc, we want to publish module.modulemap next to the fluentui-swift.h * Removing spacing and padding values from token sets (#1220) * First set of controls without spacing tokens * Complete the rest of the controls * Restoring in-use values * Move layout constants to TokenSets * Revert Notification Colors to Fluent 1 Tokens (#1226) * CommandBar: Update group spacing and add large content viewer support (#1229) * Decrease CommandBar item group spacing to 8. Change received approval from the Fluent Design team. * Enable LargeContentViewer for the CommandBar * Instantiating state variables in HUD (#1233) * added min button heights for small and large sizes * Update AttributedText to scale (#1234) * no longer need .ado folder (#1232) * Bumping FluentUI version (0.8.0) (#1230) * sanitization. Addressed PR Comments * Fix RTL behavior for CommandBar Pinned Items (#1236) It was discovered that the RTL behavior for the pinned items in CommandBar was off. The leading and trailing groups were switching sides as expected, but the additional transform being applied `configureHierarchy` is causing the views inside the group to be shown in LTR order. Fix: Remove the transform for the leading and trailing groups inside `configureHierarchy` and let the default behavior from the UIStackView handle the transition. * Update Avatar size ramp (#1194) * Renamed avatar sizes * Added avatar size 20 and size 56 * Added new sizes in demo controllers * Removed unused variable in AvatarGroupDemoController * Moved changes to token sets * Moved size tokens outside of AvatarTokenSet to fix conflicts * Bottom sheet collapsed height resolver API (#1207) * Revert "Revert "PillButtonBar: Fixing warnings for iOS 15 target deployment version. (#1165)" (#1213)" (#1244) This reverts commit 518bdc6. * Add swipe to hide API to bottom sheet (#1242) * Resolved merge and build errors * Removing unnecessary AvatarTokens.swift * Trailing whitespace Co-authored-by: Harrie Shin <hyshin@microsoft.com> Co-authored-by: isatoderici <65185855+isatoderici@users.noreply.github.com> Co-authored-by: Amogh Vinod Aralimatti <amaralim@microsoft.com> Co-authored-by: Mike Schreiber <mischreiber@microsoft.com> Co-authored-by: huwilkes <67026548+huwilkes@users.noreply.github.com> Co-authored-by: Jeanie Huynh <31874971+jeaniehuynh@users.noreply.github.com> Co-authored-by: Des Marks <johnmarks@microsoft.com> Co-authored-by: Sophia Lee <sophia.lee0416@gmail.com> Co-authored-by: Amogh Vinod Aralimatti <moggogogo@gmail.com> Co-authored-by: Lamine Male <106181067+laminesm@users.noreply.github.com> Co-authored-by: Lukas Capkovic <3610850+lcapkovic@users.noreply.github.com>
let oldContext = lastCollapsedSheetHeightResolutionContext | ||
let newContext = ContentHeightResolutionContext(maximumHeight: maxSheetHeight - view.safeAreaInsets.bottom, containerTraitCollection: view.traitCollection) | ||
|
||
if oldContext?.maximumHeight != newContext.maximumHeight || !(oldContext?.containerTraitCollection.containsTraits(in: newContext.containerTraitCollection) ?? false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm just weak, but optional chaining combined with a boolean operator (||
) really confuses me. This seems like a good case for a
if let oldContext = oldContext,
oldContext.maximumHeight != newContext.maximumHeight || !oldContext.containerTraitCollection.containsTraits(in: newContext.containerTraitCollection) {
}
Platforms Impacted
Description of changes
It's a common scenario for clients to dynamically adjust the bottom sheet collapsed height. Typically these decisions depend on:
Doing this well can have caveats, like ensuring the height change is properly timed and animated. We can make this easier for clients by providing a height resolver API. The clients just provide a closure that gets the necessary context as input (max height and current trait collection) and returns the desired height given the context. The bottom sheet ensures the closure is called at the right times.
This way clients can ensure the sheet is sized properly without having to hook up to lifecycle events. This pattern is similar to Apple's detent height resolver - https://developer.apple.com/documentation/uikit/uisheetpresentationcontroller/detent/3976719-custom
Implementation details
The collapsed sheet height is in a hot code path that gets called a lot during layout and animations, so we cache the resolved height values to ensure we only call the client when necessary.
I split out the
ContentHeightResolutionContext
away from the bottom sheet to make it a bit more generic. There's no need to tie this pattern to the bottom sheet specifically.Verification
Test app sanity on trait collection changes.
Here's the demo app behavior on iPhone. Trait collection changes when screen is rotated, so a new height gets resolved from the client (demo controller is the client in this case):
Screen.Recording.2022-09-13.at.4.23.17.PM.mov
On iPad the sheet configured with the same resolver maintains the same height in all orientations, because the trait collection stays the same:
Screen.Recording.2022-09-13.at.4.25.18.PM.mov
Pull request checklist
This PR has considered:
Microsoft Reviewers: Open in CodeFlow