[webkit-reviews] review granted: [Bug 170090] Web Inspector: WebSockets: Messages log should remain being scrolled to the bottom when a new message is added : [Attachment 305527] Patch

[bugzilla-daemon at webkit.org]

Matt Baker <mattbaker at apple.com> has granted Nikita Vasilyev
<nvasilyev at apple.com>'s request for review:
Bug 170090: Web Inspector: WebSockets: Messages log should remain being
scrolled to the bottom when a new message is added
https://bugs.webkit.org/show_bug.cgi?id=170090

Attachment 305527: Patch

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




--- Comment #2 from Matt Baker <mattbaker at apple.com> ---
Comment on attachment 305527
  --> https://bugs.webkit.org/attachment.cgi?id=305527
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=305527&action=review

r=me, with some style suggestions.

> Source/WebInspectorUI/ChangeLog:16
> +	   Batch WebSocketContentView DOM modifications using
requestAnimationFrames.

"requestAnimationFrame" should be singular.

> Source/WebInspectorUI/UserInterface/Views/WebSocketContentView.js:103
> +	   this._scheduledUpdateFramesIdentifier = requestAnimationFrame(() =>
this._updateFrames());

Utilities.js defines Object.onNextFrame, which does exactly this! You could
probably switch to that and remove a bunch of boilerplate.

For an example see TreeElement.js:194.

> Source/WebInspectorUI/UserInterface/Views/WebSocketContentView.js:145
> +	       isText: opcode ===
WebInspector.WebSocketResource.OpCodes.TextFrame,

Since its value is used above, `isText` could be a stand-alone variable,
defined and initialized above `nodeText`:

let isText = opcode === WebInspector.WebSocketResource.OpCodes.TextFrame;
let nodeText = isText ? data :
WebInspector.WebSocketContentView.textForOpcode(opcode);

let attributes = {isOutgoing, isText};
...