[webkit-reviews] review denied: [Bug 192527] Web Inspector: provide a way to make searches case sensitive or use a regular expression : [Attachment 356875] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 12 17:08:30 PST 2018


Joseph Pecoraro <joepeck at webkit.org> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 192527: Web Inspector: provide a way to make searches case sensitive or use
a regular expression
https://bugs.webkit.org/show_bug.cgi?id=192527

Attachment 356875: Patch

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




--- Comment #4 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 356875
  --> https://bugs.webkit.org/attachment.cgi?id=356875
Patch

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

Awesome! r- for now because we should have tests for the regex / case-sensitive
functionality, which I'm sure we don't have tests for right now.

> Source/WebInspectorUI/UserInterface/Views/NavigationSidebarPanel.js:37
> +	   this._filtersSetting = new WI.Setting(identifier +
"-navigation-sidebar-filters", {});
> +	   this._filterSearchSettings =
WI.SearchUtilities.createSettings("navigation-sidebar", {
> +	       handleChanged: (event) => {
> +		   this.updateFilter();
> +	       },
> +	   });

I feel like filter bars should be very basic always case insensitive and
straightforward. I don't really have a reasoning for this though, but it would
reduce the number of settings gears all over the place. So I think we shouldn't
add them to NavigationSidebarPanel filter fields.

> Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js:34
> +		   this.focusSearchField(true);

Nice!


More information about the webkit-reviews mailing list