[webkit-reviews] review granted: [Bug 191044] Web Inspector: Audit: attempt to re-link DOM nodes for imported results : [Attachment 353454] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 191044: Web Inspector: Audit: attempt to re-link DOM nodes for imported
results
https://bugs.webkit.org/show_bug.cgi?id=191044

Attachment 353454: Patch

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




--- Comment #3 from Brian Burg <bburg at apple.com> ---
Comment on attachment 353454
  --> https://bugs.webkit.org/attachment.cgi?id=353454
Patch

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

r=me. Please rebase and run through EWS before landing.

> Source/WebInspectorUI/UserInterface/Models/AuditTestCaseResult.js:79
> +	   if (window.DOMAgent && Array.isArray(data.domNodes)) {

I don't think we should be showing audits tab if DOMAgent is unavailable (i.e.
JSContext inspection). So this check seems unnecessary.

> Source/WebInspectorUI/UserInterface/Models/AuditTestCaseResult.js:87
> +	       }));

Very nice.

> Source/WebInspectorUI/UserInterface/Models/AuditTestGroup.js:60
> +	   tests = await Promise.all(tests.map(async (test) => await
WI.AuditTestCase.fromPayload(test) || await
WI.AuditTestGroup.fromPayload(test)));

Could you add some newlines here? It's difficult to follow.

> Source/WebInspectorUI/UserInterface/Models/AuditTestGroupResult.js:55
> +	   results = await Promise.all(results.map(async (test) => await
WI.AuditTestCaseResult.fromPayload(test) || await
WI.AuditTestGroupResult.fromPayload(test)));

Could you add some newlines here? It's difficult to follow.