Created attachment 144055 [details] Tabbed settings screen, "settings" tab Combine settings screen and shortcuts screen to a single tabbed settings screen. This will make shortcuts screen more discoverable.
Created attachment 144056 [details] Tabbed settings screen, "shortcuts" tab
Created attachment 144300 [details] Patch
Comment on attachment 144300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144300&action=review Generally looks good, but see below for a few nits. > Source/WebCore/inspector/front-end/HelpScreen.js:61 > + _createCloseButton: function() { style: { needs to be on a line by its own. > Source/WebCore/inspector/front-end/SettingsScreen.js:62 > + function appendSection(name) { style: { => new line > Source/WebCore/inspector/front-end/SettingsScreen.js:140 > + _getTabbedPane: function() _getOrCreateTabbedPane > Source/WebCore/inspector/front-end/ShortcutsScreen.js:62 > + view.element.className = "help-content"; > + > + var container = view.element.createChild("div", "help-container"); Can help container be merged into the view element (i.e. help-content)?
Comment on attachment 144300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144300&action=review >> Source/WebCore/inspector/front-end/HelpScreen.js:61 >> + _createCloseButton: function() { > > style: { needs to be on a line by its own. Fixed. >> Source/WebCore/inspector/front-end/SettingsScreen.js:62 >> + function appendSection(name) { > > style: { => new line Fixed. >> Source/WebCore/inspector/front-end/SettingsScreen.js:140 >> + _getTabbedPane: function() > > _getOrCreateTabbedPane Done. >> Source/WebCore/inspector/front-end/ShortcutsScreen.js:62 >> + var container = view.element.createChild("div", "help-container"); > > Can help container be merged into the view element (i.e. help-content)? No problems.
Created attachment 144448 [details] Patch
Comment on attachment 144448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144448&action=review LGTM > Source/WebCore/inspector/front-end/ShortcutsScreen.js:63 > + var container = view.element; > + container.className = "help-content"; > + container.addStyleClass("help-container"); Just view.element.className = "help-content help-container".
Comment on attachment 144448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144448&action=review > Source/WebCore/inspector/front-end/SettingsScreen.js:639 > +WebInspector.SettingsScreen.SettingsTab = "settings"; WebInspector.SettingsScreen.Tabs = { Settings: "Settings", Shortcuts: "Shortcuts" } like we do with constants in other places. > Source/WebCore/inspector/front-end/SettingsScreen.js:645 > + * @extends {WebInspector.TabbedPane} @extends should go right after @constructor, the general rule is to mind alphabetic order of @<name>
Comment on attachment 144448 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144448&action=review >> Source/WebCore/inspector/front-end/SettingsScreen.js:639 >> +WebInspector.SettingsScreen.SettingsTab = "settings"; > > WebInspector.SettingsScreen.Tabs = { > Settings: "Settings", > Shortcuts: "Shortcuts" > } > like we do with constants in other places. Good idea, done. >> Source/WebCore/inspector/front-end/SettingsScreen.js:645 >> + * @extends {WebInspector.TabbedPane} > > @extends should go right after @constructor, the general rule is to mind alphabetic order of @<name> Fixed. >> Source/WebCore/inspector/front-end/ShortcutsScreen.js:63 >> + container.addStyleClass("help-container"); > > Just view.element.className = "help-content help-container". Done.
Created attachment 144468 [details] Patch
Committed r118747: <http://trac.webkit.org/changeset/118747>