Bug 214195

Summary: Add OK button to Date/Time form controls.
Product: WebKit Reporter: Megan Gardner <megan_gardner>
Component: New BugsAssignee: Megan Gardner <megan_gardner>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, darin, sam, thorton, webkit-bot-watchers-bugzilla, wenson_hsieh, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=214517
Bug Depends on: 214518    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing
none
Patch
none
Patch for landing none

Description Megan Gardner 2020-07-10 12:36:04 PDT
Add OK button to Date/Time form controls.
Comment 1 Megan Gardner 2020-07-10 16:19:45 PDT
Created attachment 404017 [details]
Patch
Comment 2 Megan Gardner 2020-07-10 16:22:02 PDT
<rdar://problem/65006000>
Comment 3 Megan Gardner 2020-07-10 16:24:11 PDT
Created attachment 404020 [details]
Patch
Comment 4 Sam Weinig 2020-07-10 16:24:31 PDT
Comment on attachment 404017 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404017&action=review

> Source/WebKit/ChangeLog:10
> +        Add 'OK' button to add clarity to UI, and increase the default size of the control to
> +        ensure space for both buttons. Also fixed a crash that happened when closing the control before
> +        the animation completed.

Please add tests for all these things.
Comment 5 Megan Gardner 2020-07-13 15:36:56 PDT
Created attachment 404188 [details]
Patch
Comment 6 Darin Adler 2020-07-13 15:55:08 PDT
Comment on attachment 404188 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=404188&action=review

> Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm:86
> +    auto size =  [view systemLayoutSizeFittingSize:UILayoutFittingCompressedSize];

Extra space after "=" here.

> Source/WebKit/UIProcess/ios/forms/WKDateTimeInputControl.mm:88
> +    if (size.width < 250)
> +        size.width = 250;

std::max might be a nicer way to write this.
Comment 7 Megan Gardner 2020-07-17 17:56:39 PDT
Created attachment 404626 [details]
Patch for landing
Comment 8 Megan Gardner 2020-07-17 18:45:31 PDT
Created attachment 404627 [details]
Patch for landing
Comment 9 EWS 2020-07-17 19:07:05 PDT
Committed r264563: <https://trac.webkit.org/changeset/264563>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404627 [details].
Comment 10 Aakash Jain 2020-07-18 05:16:33 PDT
> Committed r264563: <https://trac.webkit.org/changeset/264563>
This broke the watchOS build. See Bug 214517. 

The latest patch wasn't run through the EWS. Please make sure to run the patches through EWS before landing. Commit-queue only builds and tests for macOS.
Comment 11 Yusuke Suzuki 2020-07-18 05:20:20 PDT
Re-opened since this is blocked by bug 214518
Comment 12 Megan Gardner 2020-07-20 13:49:02 PDT
Created attachment 404755 [details]
Patch
Comment 13 Megan Gardner 2020-07-20 15:52:40 PDT
Created attachment 404772 [details]
Patch for landing
Comment 14 EWS 2020-07-20 16:48:45 PDT
Committed r264637: <https://trac.webkit.org/changeset/264637>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404772 [details].