[webkit-reviews] review granted: [Bug 218189] [Contact Picker API] Add support for picker UI on iOS : [Attachment 412760] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Aditya Keerthi
<akeerthi at apple.com>'s request for review:
Bug 218189: [Contact Picker API] Add support for picker UI on iOS
https://bugs.webkit.org/show_bug.cgi?id=218189

Attachment 412760: Patch

https://bugs.webkit.org/attachment.cgi?id=412760&action=review




--- Comment #15 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 412760
  --> https://bugs.webkit.org/attachment.cgi?id=412760
Patch

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

r=me, nice work :)

> Source/WebKit/UIProcess/Cocoa/WKContactPicker.mm:199
> +    Vector<WebCore::ContactInfo> info;
> +    for (CNContact *contact in contacts)
> +	   info.append([self _contactInfoFromCNContact:contact]);

Is it possible to use `makeVector` instead?

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:6856
> +    [_contactPicker presentWithRequestData:requestData
completionHandler:WTFMove(completionHandler)];

the type of `completionHandler` doesn't match what's expected by
`WKContactPicker` (i'm surprised this even compiles)

> LayoutTests/contact-picker/contacts-select-while-presenting-picker.html:27
> +		   const promise1 = new Promise((resolve, reject) => {
> +		     navigator.contacts.select(['name']).then(resolve)
> +							.catch(e =>
reject(e.message));
> +		   });

NIT: you could simplify this
```
    let promise1 = navigator.contacts.select(['name']).catch((error) => { throw
error.message; });
```