[Webkit-unassigned] [Bug 188669] [Datalist][iOS] Display suggestions for input[type=color]

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Aug 17 14:53:43 PDT 2018


Tim Horton <thorton at apple.com> changed:

           What    |Removed                     |Added
 Attachment #347281|review?                     |review+
              Flags|                            |

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

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

> Source/WebKit/UIProcess/ios/forms/WKFormColorPicker.mm:48

Newline before the #if usually

> Source/WebKit/UIProcess/ios/forms/WKFormColorPicker.mm:168
> +    return @[ @[ [UIColor redColor], [UIColor orangeColor], [UIColor yellowColor], [UIColor greenColor], [UIColor cyanColor], [UIColor blueColor], [UIColor magentaColor], [UIColor purpleColor], [UIColor brownColor], [UIColor whiteColor], [UIColor grayColor], [UIColor blackColor] ] ];

Ideally I think these would have all been UIColor.redColor (dots!), but whatever.

> Source/WebKit/UIProcess/ios/forms/WKFormColorPicker.mm:202
> +            WebCore::Color color = view.assistedNodeInformation.suggestedColors.at(i);

Why at() instead of subscripting? (maybe there's a reason?)

> Source/WebKit/UIProcess/ios/forms/WKFormColorPicker.mm:203
> +            [colors addObject: [UIColor colorWithCGColor:cachedCGColor(color)]];

There's some weird spaces here (after the colon).
Do we not have a UIColor cache? (maybe not)

> Source/WebKit/UIProcess/ios/forms/WKFormColorPicker.mm:208
> +    } else
> +#endif
> +        topColorMatrix = [[self class] defaultTopColorMatrix];

I do not love flow-control-crossing-ifdef-boundaries, but maybe it's fine.

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/20180817/47454ad2/attachment.html>

More information about the webkit-unassigned mailing list