[webkit-reviews] review granted: [Bug 212064] Web Inspector: Left/Right arrow keys should collapse/expand details sections : [Attachment 399715] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 212064: Web Inspector: Left/Right arrow keys should collapse/expand details
sections
https://bugs.webkit.org/show_bug.cgi?id=212064

Attachment 399715: Patch

https://bugs.webkit.org/attachment.cgi?id=399715&action=review




--- Comment #4 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 399715
  --> https://bugs.webkit.org/attachment.cgi?id=399715
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)