Bug 212064

Summary: Web Inspector: Left/Right arrow keys should collapse/expand details sections
Product: WebKit Reporter: Nikita Vasilyev <nvasilyev>
Component: Web InspectorAssignee: 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 Flags
Patch
hi: review+
[Video] With patch applied
none
Patch none

Nikita Vasilyev
Reported 2020-05-19 00:13:49 PDT
Steps: 1. Open Elements tab 2. Select Computed tab 3. Focus on "Box Model" section header 4. Press Arrow Right Expected: Section expands. Actual: Nothing happens. Notes: In DOM tree outline, left/right keys collapse/expand the selected tree branch. However, the details sections currently don't collapse/expand by pressing left/right keys.
Attachments
Patch (5.37 KB, patch)
2020-05-19 00:24 PDT, Nikita Vasilyev 		hi: review+
DetailsFormatted DiffDiff
[Video] With patch applied (484.92 KB, video/quicktime)
2020-05-19 00:24 PDT, Nikita Vasilyev 		no flags
Details
Patch (5.46 KB, patch)
2020-05-20 14:36 PDT, Nikita Vasilyev 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-05-19 00:23:57 PDT
<rdar://problem/63384091>
Nikita Vasilyev
Comment 2 2020-05-19 00:24:03 PDT
Created attachment 399715 [details] Patch
Nikita Vasilyev
Comment 3 2020-05-19 00:24:42 PDT
Created attachment 399716 [details] [Video] With patch applied
Devin Rousso
Comment 4 2020-05-20 10:50:28 PDT
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)
Nikita Vasilyev
Comment 5 2020-05-20 14:35:57 PDT
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.
Nikita Vasilyev
Comment 6 2020-05-20 14:36:26 PDT
Created attachment 399893 [details] Patch
EWS
Comment 7 2020-05-20 15:08:18 PDT
Committed r261962: <https://trac.webkit.org/changeset/261962> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399893 [details].
Note You need to log in before you can comment on or make changes to this bug.