[webkit-reviews] review denied: [Bug 195777] Web Inspector: provide a way to reset all settings : [Attachment 364885] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 20 09:16:18 PDT 2019

Timothy Hatcher <timothy at apple.com> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 195777: Web Inspector: provide a way to reset all settings

Attachment 364885: Patch


--- Comment #10 from Timothy Hatcher <timothy at apple.com> ---
Comment on attachment 364885
  --> https://bugs.webkit.org/attachment.cgi?id=364885

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

I think we need a dialog UI for Web Inspector — maybe WI.Modal is that but as
WI.ModalDialog. Blurring the entire window is not ideal and not something found
in other apps. This would be a dialog with Reset and Cancel buttons. Having one
button makes it hard to reason how to exit without resetting.

> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:261
> +	       resetModalButton.addEventListener("click", (event) => {

This handler is big for being inline. I would expect this to be moved to a
member function. It also is more Manager like than View like code. Main.js
would be a better place for this logic to be maintained.

More information about the webkit-reviews mailing list