Summary: | Web Inspector: Add a setting for clearing the console on page reload | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ronald Jett <rjett> | ||||||||
Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Enhancement | CC: | commit-queue, graouts, joepeck, timothy, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Ronald Jett
2014-06-27 15:04:38 PDT
Created attachment 234719 [details]
patch
Comment on attachment 234719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=234719&action=review > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:113 > get navigationItems() > { > - return [this._searchBar, this._scopeBar, this._clearLogNavigationItem, this._toggleSplitNavigationItem]; > + return [this._searchBar, this._scopeBar, this._clearLogNavigationItem, this._clearLogOnReloadNavigationItem, this._toggleSplitNavigationItem]; I don't think a navigation bar item is the right UI for this at this time. (We would need a new icon that is not confusing with a general Reload.) Lets add it to the context menu. You can do that in _handleContextMenuEvent by using appendCheckboxItem. You should also do appendSeparator between the existing Clear Log item. > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:301 > + if (this._clearLogOnReloadSetting.value) { > + this._clearLog(); > + } WebKit style is to not use braces for single lines. Created attachment 246915 [details]
patch
I updated the code, taking in to account your feedback. Sorry it took so long. Let me know what you think. Comment on attachment 246915 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=246915&action=review > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:75 > + this._clearLogOnReloadSetting = new WebInspector.Setting("clear-log-on-reload", false); We might want to default it to true. > Source/WebInspectorUI/UserInterface/Views/LogContentView.js:343 > + var clearLogOnReloadUIString = WebInspector.UIString("Clear Log on Reload"); > + > + if (this._clearLogOnReloadSetting.value) > + clearLogOnReloadUIString = WebInspector.UIString("Keep Log on Reload"); Nit: Could be one line: this._clearLogOnReloadSetting.value ? WebInspector.UIString("Keep Log on Reload") : WebInspector.UIString("Clear Log on Reload"); Created attachment 246975 [details]
patch
Updated the if to be a ternary. Switching the setting to be true by default. Is there a WebKit JS style guide somewhere? > Is there a WebKit JS style guide somewhere? This gets occasional updates: http://trac.webkit.org/wiki/WebInspectorCodingStyleGuide Comment on attachment 246975 [details] patch Clearing flags on attachment: 246975 Committed r180432: <http://trac.webkit.org/changeset/180432> All reviewed patches have been landed. Closing bug. |