| Summary: | Web Inspector: Left/Right arrow keys should collapse/expand details sections | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikita Vasilyev <nvasilyev> | ||||||||
| Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | hi, inspector-bugzilla-changes, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Nikita Vasilyev
2020-05-19 00:13:49 PDT
Created attachment 399715 [details]
Patch
Created attachment 399716 [details]
[Video] With patch applied
Comment on attachment 399715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399715&action=review r=me > Source/WebInspectorUI/UserInterface/Views/DetailsSection.js:40 > + this._headerElement.addEventListener("keydown", this._headerElementKeyDown.bind(this)); NIT: I've been trying to move away from `_on*` method names, as I find `_handle*` to be clearer. > Source/WebInspectorUI/UserInterface/Views/DetailsSection.js:161 > + if (this._optionsElement?.contains(event.target)) > + return; NIT: this could probably be earlier given that it doesn't care about `isSpaceOrEnterKey`. > Source/WebInspectorUI/UserInterface/Views/DetailsSection.js:166 > + this.collapsed = !this.collapsed; NIT: early `return`? > Source/WebInspectorUI/UserInterface/Views/ExpandableView.js:35 > + this._disclosureButton.addEventListener("keydown", this._onDisclosureButtonKeyDown.bind(this)); Ditto (DetailsSection.js:40) Comment on attachment 399715 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399715&action=review >> Source/WebInspectorUI/UserInterface/Views/DetailsSection.js:161 >> + return; > > NIT: this could probably be earlier given that it doesn't care about `isSpaceOrEnterKey`. Yeah, but I intentionally put it here because because `contains` involves DOM traversal. I'd rather not do it on every keydown. Created attachment 399893 [details]
Patch
Committed r261962: <https://trac.webkit.org/changeset/261962> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399893 [details]. |