Summary: | Web Inspector: Add keyboard shortcut for switching to last used dock configuration | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Masataka Yakura <myakura.web> | ||||||||||
Component: | Web Inspector | Assignee: | Devin Rousso <hi> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | apavlov, bburg, commit-queue, hi, joepeck, keishi, loislo, pfeldman, pmuellr, syoichi, timothy, vsevik, web-inspector-bugs, webkit-bug-importer, yurys | ||||||||||
Priority: | P2 | Keywords: | EasyFix, InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Attachments: |
|
Description
Masataka Yakura
2013-02-20 04:58:16 PST
Fixing component Created attachment 287680 [details]
Patch
Comment on attachment 287680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287680&action=review Thanks for switching to the enums. But please change it to the simpler "toggleDockSide" strategy, and clarify what happen when not docked in the ChangeLog. > Source/WebInspectorUI/ChangeLog:7 > + Does it affect docking at all if the inspector is not attached? > Source/WebInspectorUI/UserInterface/Base/Main.js:319 > + this._switchToPreviousDockSideKeyboardShortcut = new WebInspector.KeyboardShortcut(WebInspector.KeyboardShortcut.Modifier.CommandOrControl | WebInspector.KeyboardShortcut.Modifier.Shift, "D", this._switchToPreviousDockSide.bind(this)); There are only two dock sides, so this should be called toggleDockSideKeyboardShortcut. And similar for the other parts of the patch. I don't think we even need to keep track of previous dock side, just switch it if docked already. Comment on attachment 287680 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287680&action=review >> Source/WebInspectorUI/ChangeLog:7 >> + > > Does it affect docking at all if the inspector is not attached? Yes. I will rename this bug so that it better clarifies what this patch does. The idea is that pressing Cmd+Shift+D will toggle the dock "configuration" to whatever was previously used. This means that if WebInspector is docked right, and you decide to switch to docked bottom, pressing Cmd+Shift+D will toggle between right and bottom. This also applies to undocked (e.g. if you are docked right and switch to undocked, Cmd+Shift+D will toggle between right and undocked). >> Source/WebInspectorUI/UserInterface/Base/Main.js:319 >> + this._switchToPreviousDockSideKeyboardShortcut = new WebInspector.KeyboardShortcut(WebInspector.KeyboardShortcut.Modifier.CommandOrControl | WebInspector.KeyboardShortcut.Modifier.Shift, "D", this._switchToPreviousDockSide.bind(this)); > > There are only two dock sides, so this should be called toggleDockSideKeyboardShortcut. And similar for the other parts of the patch. I don't think we even need to keep track of previous dock side, just switch it if docked already. See above. I will rename this to _togglePreviousDockConfigurationKeyboardShortcut. Created attachment 287814 [details]
Patch
Comment on attachment 287814 [details] Patch Attachment 287814 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1996442 New failing tests: fast/dom/HTMLFormElement/document-deactivation-callback-crash.html Created attachment 287823 [details]
Archive of layout-test-results from ews113 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 287814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287814&action=review r=me. I found a typo. > Source/WebInspectorUI/ChangeLog:25 > + - Left I think this should say 'Bottom' > Source/WebInspectorUI/ChangeLog:26 > + - Right At some point we may change this to "trailing" if we support RTL, but don't worry for now. Created attachment 287875 [details]
Patch
Comment on attachment 287875 [details] Patch Clearing flags on attachment: 287875 Committed r205413: <http://trac.webkit.org/changeset/205413> All reviewed patches have been landed. Closing bug. |