Bug 149608 – Dashboard metrics has JS errors from toggle button patch

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dean Johnson]

The Dashboard metrics page has JS errors that were introduced with https://bugs.webkit.org/show_bug.cgi?id=148403. This patch fixes them and also moves some of the identical logic from dashboard/Scripts/Main.js and dashboard/Scripts/MetricsMain.js to dashboard/Scripts/Settings.js.

[Dean Johnson]

Created attachment 262025 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 262025 [details]
Patch

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

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/MetricsMain.js:346
> +        var settingsWrapper = document.createElement("div");

Eventually we should refactor this to have less copy/pasted code.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Settings.js:91
> +        if (platformName)
> +            return platformName.substr(0, platformName.indexOf("-"));
> +        return ''

Coding style nit: we prefer early return, so this should be

if (!platformName)
    return ''
return platformName.substr(0, platformName.indexOf("-"));

[Dean Johnson]

Created attachment 262026 [details]
Patch

[Dean Johnson]

(In reply to comment #2)
> Comment on attachment 262025 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=262025&action=review
> 
> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/MetricsMain.js:346
> > +        var settingsWrapper = document.createElement("div");
> 
> Eventually we should refactor this to have less copy/pasted code.
> 
Agreed.

> > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Settings.js:91
> > +        if (platformName)
> > +            return platformName.substr(0, platformName.indexOf("-"));
> > +        return ''
> 
> Coding style nit: we prefer early return, so this should be
> 
> if (!platformName)
>     return ''
> return platformName.substr(0, platformName.indexOf("-"));
Fixed in latest patch.

[WebKit Commit Bot]

Comment on attachment 262026 [details]
Patch

Clearing flags on attachment: 262026

Committed r190303: <http://trac.webkit.org/changeset/190303>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.