[webkit-reviews] review granted: [Bug 187741] [Datalist] Add button to TextFieldInputs with a datalist : [Attachment 345253] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Aditya Keerthi
<akeerthi at apple.com>'s request for review:
Bug 187741: [Datalist] Add button to TextFieldInputs with a datalist
https://bugs.webkit.org/show_bug.cgi?id=187741

Attachment 345253: Patch

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




--- Comment #8 from Tim Horton <thorton at apple.com> ---
Comment on attachment 345253
  --> https://bugs.webkit.org/attachment.cgi?id=345253
Patch

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

> Source/WebCore/rendering/RenderThemeMac.mm:206
> +- (void)drawWithFrame:(NSRect)cellFrame inView:(__unused NSView
*)controlView

A bit sad that this can’t just use a system cell.

> Source/WebCore/rendering/RenderThemeMac.mm:226
> +    if ([self userInterfaceLayoutDirection] ==
NSUserInterfaceLayoutDirectionRightToLeft) {

Does something really propagate UILayoutDirection into here!?

> Source/WebCore/rendering/RenderThemeMac.mm:230
> +	   [transform translateXBy:-1*NSMidX(cellFrame)
yBy:-1*NSMidY(cellFrame)];

Spaces around operators

> Source/WebCore/rendering/RenderThemeMac.mm:237
> +    if (accentColor == NSUserAccentColorRed)

I continue to not love this.

> Source/WebCore/rendering/RenderThemeMac.mm:261
> +    RefPtr<WebCore::Image> image =
WebCore::Image::loadPlatformResource("ListButtonArrow");

Much better!

> Source/WebCore/rendering/RenderThemeMac.mm:1085
> +    NSRect listButtonFrame = NSMakeRect(r.maxX() - 16, r.y(), 16,
r.height());

What’s this random 16? Would be nice to have a named constant.