Bug 160054 - Web Inspector: Console log counter on the dashboard should be better at displaying large numbers
Summary: Web Inspector: Console log counter on the dashboard should be better at displ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: EasyFix, GoodFirstBug, InRadar
Depends on:
Blocks:
 
Reported: 2016-07-21 15:33 PDT by Nikita Vasilyev
Modified: 2016-08-19 14:04 PDT (History)
8 users (show)

See Also:


Attachments
[Image] Current behavior (14.58 KB, image/png)
2016-07-21 15:33 PDT, Nikita Vasilyev
no flags Details
Patch (2.22 KB, patch)
2016-08-18 23:09 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (3.06 KB, patch)
2016-08-19 09:18 PDT, Devin Rousso
mattbaker: review+
Details | Formatted Diff | Diff
Patch (3.96 KB, patch)
2016-08-19 11:10 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2016-07-21 15:33:03 PDT
Created attachment 284271 [details]
[Image] Current behavior

When there are >=1000 messages in the console, it is always shown as "999+".

Instead, we should show:
2000 as 2k
2500 as 2.5k
2542 as 2.5k
1000000 as 1m
Comment 1 Radar WebKit Bug Importer 2016-07-21 15:33:30 PDT
<rdar://problem/27481266>
Comment 2 Devin Rousso 2016-08-18 23:09:13 PDT
Created attachment 286441 [details]
Patch
Comment 3 Matt Baker 2016-08-19 01:26:25 PDT
Comment on attachment 286441 [details]
Patch

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

This could be a utility function, like Number.bytesToString:

Object.defineProperty(Number, "abbreviate",
{
    ...
});

> Source/WebInspectorUI/UserInterface/Views/DefaultDashboardView.js:104
> +            return `${Math.round(number / 100) / 10}k`;

I think uppercase K, M, and B are more common. Although these pseudo-standard metric abbreviations are widely used, they should be UIStrings to allow localization.

> Source/WebInspectorUI/UserInterface/Views/DefaultDashboardView.js:109
> +        return `${Math.round(number / 100000000) / 10}b`;

That's a lot of console messages!
Comment 4 Devin Rousso 2016-08-19 09:18:05 PDT
Created attachment 286455 [details]
Patch
Comment 5 Matt Baker 2016-08-19 11:05:18 PDT
Comment on attachment 286455 [details]
Patch

r=me

Let's remove _formatPossibleLargeNumber and just call Number.abbreviate directly in the two places it's used.
Comment 6 Devin Rousso 2016-08-19 11:10:48 PDT
Created attachment 286463 [details]
Patch
Comment 7 WebKit Commit Bot 2016-08-19 11:42:46 PDT
Comment on attachment 286463 [details]
Patch

Clearing flags on attachment: 286463

Committed r204642: <http://trac.webkit.org/changeset/204642>
Comment 8 WebKit Commit Bot 2016-08-19 11:42:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Nikita Vasilyev 2016-08-19 14:04:04 PDT
Comment on attachment 286463 [details]
Patch

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

Looks good!

> Source/WebInspectorUI/UserInterface/Base/Utilities.js:1047
> +            return WebInspector.UIString("%.1fK").format(Math.round(num / 100) / 10);

1000 is shown as "1.0K" and not "1K".
It's better in the case when the counter changes and the width stays the same.