[Webkit-unassigned] [Bug 186531] [Datalist][macOS] Add suggestions UI for TextFieldInputTypes

[bugzilla-daemon at webkit.org]

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

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

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

> Source/WebKit/UIProcess/WebDataListSuggestionsDropdown.h:51
> +    virtual void close() { m_client->didCloseSuggestions(); m_client = nullptr; }

WebKit style generally doesn’t allow two statements on one line like this. Split this into multiple lines at least (and possibly also move it to the implementation? Why is it here?).

> Source/WebKit/UIProcess/WebPageProxy.cpp:7492
> +void WebPageProxy::closeOverlayedViews()

INTERESTING. I wonder if we should merge this with the dismissContentRelativeChildWindows mechanism that dismisses e.g. find highlights and force-touch previews...

> Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:50
> +- (void)setText:(NSString *)text;
> +- (void)setActive:(BOOL)shouldActivate;

I realize you don’t need getters now, but is there any reason these aren’t properties?

> Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:148
> + at implementation WKDataListSuggestionCell

Is there a reason this isn’t just NSTextFieldCell (or a subclass), instead of being our own implementation of that? Seems like that would get rid of a lot of code.

> Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:158
> +    [_textField setBackgroundColor:[NSColor clearColor]];

Does this do the right thing in Dark Mode? (I’m not actually sure what the right thing is in this case)

> Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:161
> +    [self setIdentifier:@"WKDataListSuggestionCell"];

I thought you had a constant for this!

> Source/WebKit/UIProcess/mac/WebDataListSuggestionsDropdownMac.mm:163
> +    [self addTrackingRect:NSMakeRect(0, 0, NSWidth(frameRect), NSHeight(frameRect)) owner:self userData:nil assumeInside:NO];

Isn’t this rect just self.bounds?

-- 
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/20180630/28d5b7d2/attachment.html>