Skip to content

Commit

Permalink
Fix bottom commanding bug where dimming view is still accessible when…
Browse files Browse the repository at this point in the history
… collapsed (#2072)

* Revert "Calculate Bottom Commanding header height instead of using constant (#2041)"

This reverts commit d298dac.

* change headerTopMargin

* set alpha explicitly if not transitioning
  • Loading branch information
joannaquu authored Jul 27, 2024
1 parent 411c8c5 commit 2d3a03b
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ class BottomCommandingDemoController: DemoController {

private lazy var heroItems: [CommandingItem] = {
return Array(1...25).map {
let title = ($0 == 4) ? "Two line item" : "Item"
let item = CommandingItem(title: title + String($0), image: homeImage, action: commandAction)
let item = CommandingItem(title: "Item " + String($0), image: homeImage, action: commandAction)
item.selectedImage = homeSelectedImage
item.isOn = ($0 % 3 == 1)
item.isEnabled = ($0 % 2 == 1)
Expand Down
19 changes: 8 additions & 11 deletions ios/FluentUI/Bottom Commanding/BottomCommandingController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ open class BottomCommandingController: UIViewController, TokenizedControlInterna
headerView.addSubview(heroCommandStack)

let sheetController = BottomSheetController(headerContentView: headerView, expandedContentView: makeSheetExpandedContent(with: tableView))
sheetController.headerContentHeight = Constants.BottomSheet.headerHeight
sheetController.hostedScrollView = tableView
sheetController.isHidden = isHidden
sheetController.shouldAlwaysFillWidth = sheetShouldAlwaysFillWidth
Expand Down Expand Up @@ -533,7 +534,6 @@ open class BottomCommandingController: UIViewController, TokenizedControlInterna
} else {
tableView.tableHeaderView = nil
}
calculateHeaderHeight()
}

private func reloadHeroCommandOverflowStack() {
Expand Down Expand Up @@ -563,13 +563,6 @@ open class BottomCommandingController: UIViewController, TokenizedControlInterna
}
}

@discardableResult
private func calculateHeaderHeight() -> CGFloat {
let headerHeight = heroCommandStack.systemLayoutSizeFitting(UIView.layoutFittingCompressedSize).height + BottomSheetController.resizingHandleHeight
bottomSheetController?.headerContentHeight = headerHeight
return headerHeight
}

private func updateAppearance() {
guard isViewLoaded else {
return
Expand Down Expand Up @@ -927,10 +920,10 @@ open class BottomCommandingController: UIViewController, TokenizedControlInterna
bottomSheetController.isExpandable = isExpandable

let maxHeroItemHeight = heroCommandStack.arrangedSubviews.map { $0.intrinsicContentSize.height }.max() ?? Constants.defaultHeroButtonHeight
let headerHeightWithoutBottomWhitespace = BottomCommandingTokenSet.handleHeaderHeight + maxHeroItemHeight
let headerHeightWithoutBottomWhitespace = BottomCommandingTokenSet.headerTopMargin + maxHeroItemHeight

// How much more whitespace is required at the bottom of the sheet header
let requiredBottomWhitespace = max(0, calculateHeaderHeight() - headerHeightWithoutBottomWhitespace)
let requiredBottomWhitespace = max(0, Constants.BottomSheet.headerHeight - headerHeightWithoutBottomWhitespace)

// The safe area inset can fulfill some or all of our bottom whitespace requirement.
// This is how much more we need, taking the inset into account.
Expand All @@ -940,7 +933,7 @@ open class BottomCommandingController: UIViewController, TokenizedControlInterna
let addedHeaderTopMargin = !isExpandable
? BottomSheetController.resizingHandleHeight
: 0
bottomSheetHeroStackTopConstraint?.constant = BottomCommandingTokenSet.handleHeaderHeight + addedHeaderTopMargin
bottomSheetHeroStackTopConstraint?.constant = BottomCommandingTokenSet.headerTopMargin + addedHeaderTopMargin

let oldCollapsedContentHeight = bottomSheetController.collapsedContentHeight
let newCollapsedContentHeight = headerHeightWithoutBottomWhitespace + reducedBottomWhitespace + addedHeaderTopMargin
Expand Down Expand Up @@ -1125,6 +1118,10 @@ open class BottomCommandingController: UIViewController, TokenizedControlInterna
static let moreButtonIcon: UIImage? = UIImage.staticImageNamed("more-24x24")
static let moreButtonTitle: String = "CommandingBottomBar.More".localized
}

struct BottomSheet {
static let headerHeight: CGFloat = 66
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,5 @@ extension BottomCommandingTokenSet {
static let tabVerticalPadding: CGFloat = GlobalTokens.spacing(.size80)
static let tabHorizontalPadding: CGFloat = GlobalTokens.spacing(.size160)
static let strokeWidth: CGFloat = GlobalTokens.stroke(.width05)
static let handleHeaderHeight: CGFloat = GlobalTokens.spacing(.size120)
static let headerTopMargin: CGFloat = GlobalTokens.spacing(.size60)
}
12 changes: 12 additions & 0 deletions ios/FluentUI/Bottom Sheet/BottomSheetController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,15 @@ public class BottomSheetController: UIViewController, Shadowable, TokenizedContr
targetAlpha = abs(currentOffset - lowestUndimmedOffset) / (lowestUndimmedOffset - highestDimmedOffset)
}
}

if currentExpansionState != .transitioning {
// In the case that there has been a floating point precision error and
// targetAlpha is a value very close to 0 or 1, set it explicitly
if targetAlpha != 0 || targetAlpha != 1 {
targetAlpha = currentExpansionState == .expanded ? 1 : 0
}
}

dimmingView.alpha = targetAlpha
}

Expand Down Expand Up @@ -1051,6 +1060,9 @@ public class BottomSheetController: UIViewController, Shadowable, TokenizedContr

bottomSheetView.isHidden = targetExpansionState == .hidden

updateDimmingViewAlpha()
updateDimmingViewAccessibility()

// UIKit doesn't properly handle interrupted constraint animations, so we need to
// detect and fix a possible desync here
updateSheetLayoutGuideTopConstraint()
Expand Down

0 comments on commit 2d3a03b

Please sign in to comment.