[Webkit-unassigned] [Bug 207664] WebKit support for Apple Pay Buttons with custom corner radii

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=207664

Andy Estes <aestes at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aestes at apple.com
 Attachment #390563|review?                     |review-
              Flags|                            |

--- Comment #2 from Andy Estes <aestes at apple.com> ---
Comment on attachment 390563
  --> https://bugs.webkit.org/attachment.cgi?id=390563
Patch

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

> Source/WebCore/rendering/RenderThemeCocoa.mm:112
> +        PKDrawApplePayButtonWithCornerRadius(paintInfo.context().platformContext(), CGRectMake(paintRect.x(), -paintRect.maxY(), paintRect.width(), paintRect.height()), 1.0, std::max({topLeft.height.value(), topLeft.width.value(), topRight.height.value(), topRight.width.value(), bottomLeft.height.value(), bottomLeft.width.value(), bottomRight.height.value(), bottomRight.width.value()}), toPKPaymentButtonType(renderer.style().applePayButtonType()), toPKPaymentButtonStyle(renderer.style().applePayButtonStyle()), renderer.style().locale());

Pretty hard to read all this as one statement. Can you compute the corner radius separately? Maybe a static helper function that takes a `const RenderStyle&` and returns the (maybe default) corner radius?

> Source/WebCore/rendering/RenderThemeCocoa.mm:116
>      PKDrawApplePayButton(paintInfo.context().platformContext(), CGRectMake(paintRect.x(), -paintRect.maxY(), paintRect.width(), paintRect.height()), 1.0, toPKPaymentButtonType(renderer.style().applePayButtonType()), toPKPaymentButtonStyle(renderer.style().applePayButtonStyle()), renderer.style().locale());

I would call PKDrawApplePayButtonWithCornerRadius whether or not there is a `border-radius`. If there's no border radius, you can use `PKDrawApplePayButtonWithCornerRadius` as a default (see SOFT_LINK_CONSTANT_FOR_{HEADER,SOURCE} for how to load it).

> LayoutTests/http/tests/ssl/applepay/ApplePayButton.html:39
> +        .borderRadiusLarge{

Missing a space before the {.

> LayoutTests/http/tests/ssl/applepay/ApplePayButton.html:43
> +            border-top-left-radius: 20px;
> +            border-top-right-radius: 25px;
> +            border-bottom-left-radius: 10px;
> +            border-bottom-right-radius: 5px;

How about cases where the width and height differ?

> LayoutTests/http/tests/ssl/applepay/ApplePayButton.html:50
> +        for (let style of ["white","white-outline", "black"]) {

Missing a space before "white-outline".

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200214/592c25bb/attachment-0001.htm>