Skip to content

Commit

Permalink
[iOS] Date picker is clipped in landscape mode on some narrower iPhon…
Browse files Browse the repository at this point in the history
…e models

https://bugs.webkit.org/show_bug.cgi?id=262451
rdar://116317686

Reviewed by Aditya Keerthi and Abrar Rahman Protyasha.

On iPhone models where the screen is narrower than 400 pt, focusing a date input while in landscape
orientation causes the bottom of the date picker (along with the accessory view containing "Done"
and "Reset" buttons) to become clipped. This is because, despite the fact that the date picker
popover lays out in undocked (arrow-less) mode in the center of the window, the window still isn't
tall enough to accomodate the full height of the date picker, whose layout is unable to compress
vertically to fit within the height of the window. This means that even if we adjust the height
constraint of the date picker and its containing view to be contained within the window, the date
picker will just clip itself to fit within the new height.

`_UIDatePickerOverlayPresentation` (which we stopped using in 268069@main) solved this problem by
scaling the entire content view in the popover to fit within the new size by setting a transform on
the content view containing the date picker and accessory view. We adopt a similar solution here,
and apply a transform to the `WKDatePickerPopoverView` if needed during view layout to fit within
the bounds of the view.

Note that we also need to add a new width constraint that fits the popover container view to the
content view to scale alongside the new `targetScale`; otherwise, the popover's view will be wider
than the content view itself, which leads to unnecessary left and right margins.

To test this, we:

1.  Add a layout test to rotate the device to landscape orientation and tap on a date input.
2.  Add a release assertion when simulating date picker dismissal by tapping on the Done button, to
    crash if the accessory view can't be hit-tested to (e.g. clipped).

Without this fix, we'll crash in the new release assert in `-assertAccessoryViewCanBeHitTested`.

* LayoutTests/fast/forms/ios/show-and-dismiss-date-input-in-landscape-expected.txt: Added.
* LayoutTests/fast/forms/ios/show-and-dismiss-date-input-in-landscape.html: Added.
* LayoutTests/fast/forms/ios/time-picker-value-change.html:

Adjust an existing layout test to wait for the context menu to finish presenting before interacting
with the accessory view's "Done" button, in order to avoid hitting the new assertion under
`-dismissWithAnimation`.

* Source/WebKit/UIProcess/ios/forms/WKDatePickerPopoverController.h:
* Source/WebKit/UIProcess/ios/forms/WKDatePickerPopoverController.mm:
(-[WKDatePickerPopoverView estimatedMaximumPopoverSize]):

Drive-by fix: increase the extra padding before falling back to arrow-less presentation to 80 pt; I
discovered that on an iPhone 12 with 60 pt, it's still possible to end up squishing (but not quite
totally clipping) the accessory view in some cases.

(-[WKDatePickerPopoverController assertAccessoryViewCanBeHitTestedForTesting]):

Add some logic to sanity check that the accessory view can actually be hit-tested (relative to the
popover's content view) when simulating tapping the "Done" button to dismiss the date picker.

(-[WKDatePickerPopoverController viewDidLoad]):
(-[WKDatePickerPopoverController viewWillLayoutSubviews]):
(-[WKDatePickerPopoverController _scaleDownToFitHeightIfNeeded]):

This is the main fix; see above for more details.

* Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.h:
* Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm:
(-[WKDateTimeInputControl dismissWithAnimationForTesting]):
(-[WKDateTimeInputControl dismissWithAnimation]): Deleted.

Add a `ForTesting` suffix to this testing-only method.

Canonical link: https://commits.webkit.org/268753@main
  • Loading branch information
whsieh committed Oct 2, 2023
1 parent a073a3d commit 297cfba
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
This test verifies that presenting a date picker in landscape orientation on an iPhone does not cause the Done button to be clipped.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS Presented date picker
PASS Dismissed date picker
PASS successfullyParsed is true

TEST COMPLETE

Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<script src="../../../resources/ui-helper.js"></script>
<script src="../../../resources/js-test.js"></script>
<style>
body, html {
width: 100%;
height: 100%;
margin: 0;
padding: 0;
}

input {
width: 200px;
height: 44px;
position: absolute;
left: calc(50% - 100px);
top: calc(50% - 22px);
}
</style>
</head>
<body>
<input type="datetime-local"></input>
<script>
jsTestIsAsync = true;

addEventListener("load", async () => {
description("This test verifies that presenting a date picker in landscape orientation on an iPhone does not cause the Done button to be clipped.");

await UIHelper.rotateDevice("landscape-right", true);
await UIHelper.ensurePresentationUpdate();

await UIHelper.activateElement(document.querySelector("input"));
await UIHelper.waitForContextMenuToShow();
testPassed("Presented date picker");

await UIHelper.dismissFormAccessoryView();
await UIHelper.waitForContextMenuToHide();
testPassed("Dismissed date picker");

await UIHelper.rotateDevice("portrait", true);
finishJSTest();
});
</script>
</body>
</html>
6 changes: 4 additions & 2 deletions LayoutTests/fast/forms/ios/time-picker-value-change.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
if (!window.testRunner)
return;

await UIHelper.activateElementAndWaitForInputSession(time);
await UIHelper.activateElement(time);
await UIHelper.waitForContextMenuToShow();
await UIHelper.setTimePickerValue(9, 41);
await UIHelper.dismissFormAccessoryView();
await UIHelper.waitForInputSessionToDismiss();
await UIHelper.activateElementAndWaitForInputSession(time);
await UIHelper.activateElement(time);
await UIHelper.waitForContextMenuToShow();
pickerValues = await UIHelper.timerPickerValues();

shouldBe("pickerValues.hour", "9");
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Original file line number Diff line number Diff line change
Expand Up @@ -12060,7 +12060,7 @@ - (BOOL)_allowAnimationControls
- (void)dismissFormAccessoryView
{
#if !PLATFORM(WATCHOS)
if (auto dateInput = self.dateTimeInputControl; [dateInput dismissWithAnimation])
if (auto dateInput = self.dateTimeInputControl; [dateInput dismissWithAnimationForTesting])
return;
#endif
[self accessoryDone];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
- (instancetype)initWithDatePicker:(UIDatePicker *)datePicker delegate:(id<WKDatePickerPopoverControllerDelegate>)delegate;
- (void)presentInView:(UIView *)view sourceRect:(CGRect)rect completion:(void(^)())completion;
- (void)dismissDatePicker;
- (void)assertAccessoryViewCanBeHitTestedForTesting;
@end

#endif // PLATFORM(IOS_FAMILY)
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ - (UIToolbar *)accessoryView

- (CGSize)estimatedMaximumPopoverSize
{
constexpr auto additionalHeightToAvoidClippingToolbar = 60;
static constexpr auto additionalHeightToAvoidClippingToolbar = 80;
return CGSize {
_contentSize.width,
_contentSize.height + additionalHeightToAvoidClippingToolbar
Expand All @@ -134,6 +134,8 @@ @interface WKDatePickerPopoverController () <UIPopoverPresentationControllerDele

@implementation WKDatePickerPopoverController {
RetainPtr<WKDatePickerPopoverView> _contentView;
RetainPtr<NSLayoutConstraint> _untransformedContentWidthConstraint;
RetainPtr<NSLayoutConstraint> _transformedContentWidthConstraint;
__weak id<WKDatePickerPopoverControllerDelegate> _delegate;
BOOL _canSendPopoverControllerDidDismiss;
}
Expand All @@ -155,6 +157,28 @@ - (void)resetDatePicker
[_delegate datePickerPopoverControllerDidReset:self];
}

- (void)assertAccessoryViewCanBeHitTestedForTesting
{
auto accessoryView = [_contentView accessoryView];
auto popoverView = self.viewIfLoaded;

RELEASE_ASSERT(accessoryView);
RELEASE_ASSERT(popoverView);

auto bounds = [popoverView convertRect:accessoryView.bounds fromView:accessoryView];
CGPoint center { CGRectGetMidX(bounds), CGRectGetMidY(bounds) };

BOOL hitTestedToAccessoryView = NO;
for (auto view = [popoverView hitTest:center withEvent:nil]; view; view = view.superview) {
if (view == accessoryView) {
hitTestedToAccessoryView = YES;
break;
}
}

RELEASE_ASSERT(hitTestedToAccessoryView);
}

- (void)dismissDatePicker
{
[self.presentingViewController dismissViewControllerAnimated:YES completion:[strongSelf = retainPtr(self)] {
Expand All @@ -174,7 +198,9 @@ - (void)viewDidLoad

[self.view addSubview:_contentView.get()];

_untransformedContentWidthConstraint = [[_contentView widthAnchor] constraintEqualToAnchor:self.view.widthAnchor];
[NSLayoutConstraint activateConstraints:@[
_untransformedContentWidthConstraint.get(),
[[_contentView leadingAnchor] constraintEqualToAnchor:self.view.leadingAnchor],
[[_contentView trailingAnchor] constraintEqualToAnchor:self.view.trailingAnchor],
[[_contentView topAnchor] constraintEqualToAnchor:self.view.topAnchor],
Expand All @@ -184,6 +210,45 @@ - (void)viewDidLoad
self.preferredContentSize = [_contentView systemLayoutSizeFittingSize:UILayoutFittingCompressedSize];
}

- (void)viewWillLayoutSubviews
{
[super viewWillLayoutSubviews];

if (self.beingDismissed) {
// The popover shrinks below content size while running dismissal animations.
// Don't bother shrinking the content down to fit in this case.
return;
}

[self _scaleDownToFitHeightIfNeeded];
}

- (void)_scaleDownToFitHeightIfNeeded
{
auto viewBounds = self.view.bounds;
auto originalContentFrame = [_contentView frame];
if (CGRectIsEmpty(originalContentFrame) || CGRectGetHeight(originalContentFrame) <= CGRectGetHeight(viewBounds))
return;

auto targetScale = std::min(CGRectGetWidth(viewBounds) / CGRectGetWidth(originalContentFrame), CGRectGetHeight(viewBounds) / CGRectGetHeight(originalContentFrame));
auto adjustedContentSize = CGSizeMake(CGRectGetWidth(originalContentFrame) * targetScale, CGRectGetHeight(originalContentFrame) * targetScale);

[_contentView setTransform:^{
auto translateToOriginAfterScaling = CGAffineTransformMakeTranslation(
(adjustedContentSize.width - CGRectGetWidth(originalContentFrame)) / 2,
(adjustedContentSize.height - CGRectGetHeight(originalContentFrame)) / 2
);
return CGAffineTransformScale(translateToOriginAfterScaling, targetScale, targetScale);
}()];

self.preferredContentSize = adjustedContentSize;

[_untransformedContentWidthConstraint setActive:NO];
[_transformedContentWidthConstraint setActive:NO];
_transformedContentWidthConstraint = [[_contentView widthAnchor] constraintEqualToAnchor:self.view.widthAnchor multiplier:1 / targetScale];
[_transformedContentWidthConstraint setActive:YES];
}

- (void)presentInView:(UIView *)view sourceRect:(CGRect)rect completion:(void(^)())completion
{
RetainPtr controller = [view _wk_viewControllerForFullScreenPresentation];
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
@property (nonatomic, readonly) double timePickerValueHour;
@property (nonatomic, readonly) double timePickerValueMinute;
- (void)setTimePickerHour:(NSInteger)hour minute:(NSInteger)minute;
- (BOOL)dismissWithAnimation;
- (BOOL)dismissWithAnimationForTesting;
@end

#endif // PLATFORM(IOS_FAMILY) && !PLATFORM(WATCHOS)
3 changes: 2 additions & 1 deletion Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm
Original file line number Diff line number Diff line change
Expand Up @@ -345,9 +345,10 @@ - (double)timePickerValueMinute
return -1;
}

- (BOOL)dismissWithAnimation
- (BOOL)dismissWithAnimationForTesting
{
if (auto picker = dynamic_objc_cast<WKDateTimePicker>(self.control)) {
[picker.datePickerController assertAccessoryViewCanBeHitTestedForTesting];
[picker.datePickerController dismissDatePicker];
return YES;
}
Expand Down

0 comments on commit 297cfba

Please sign in to comment.