Add symbols to the timeline dot, make them more meaningful
Created attachment 377180 [details] Patch
Created attachment 377183 [details] Symbols
Created attachment 377184 [details] Symbols
Comment on attachment 377180 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377180&action=review > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:531 > + failed: 'X', Can we use ✘ or ❌ instead? > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:563 > - if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0) > + if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0) { Can data.stats be negative? Otherwise we should be omitting > 0 per: https://webkit.org/code-style-guidelines/#zero-null
Thanks for this patch!
I'd refrain from r+ since I don't know this codebase yet.
The content of attachment 377183 [details] has been deleted
The content of attachment 377184 [details] has been deleted
(In reply to Ryosuke Niwa from comment #4) > Comment on attachment 377180 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377180&action=review > > > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:531 > > + failed: 'X', > > Can we use ✘ or ❌ instead? I would prefer ✘ so we don't have and colored unicode elements. > > > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:563 > > - if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0) > > + if (data.stats[`tests${willFilterExpected ? '_unexpected_' : '_'}${type}`] > 0) { > > Can data.stats be negative? Otherwise we should be omitting > 0 per: > https://webkit.org/code-style-guidelines/#zero-null
Comment on attachment 377180 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377180&action=review > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:529 > + const symbolMap = { We actually generate the legend in this file ( Legend(...) ) Can we define these globally and fix the scale in this patch too?
(In reply to Jonathan Bedard from comment #10) > Comment on attachment 377180 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=377180&action=review > > > Tools/resultsdbpy/resultsdbpy/view/static/js/timeline.js:529 > > + const symbolMap = { > > We actually generate the legend in this file ( Legend(...) ) Can we define > these globally and fix the scale in this patch too? Will do
Comment on attachment 377180 [details] Patch Attachment 377180 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12963467 New failing tests: js/error-should-not-strong-reference-global-object.html
Created attachment 377194 [details] Archive of layout-test-results from ews215 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews215 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Created attachment 377272 [details] Patch
Comment on attachment 377272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377272&action=review Looks really good! The only changes remaining are changing the x style (Ryosuke asked for that, I think the unicode x looks better) and making dots un-selectable. Great work! > Tools/resultsdbpy/resultsdbpy/view/static/library/css/webkit.css:2156 > + left: calc(0px - var(--tinySize)); I don't think that we want dots to be selectable.
Created attachment 377277 [details] Patch
Comment on attachment 377277 [details] Patch Clearing flags on attachment: 377277 Committed r249124: <https://trac.webkit.org/changeset/249124>
All reviewed patches have been landed. Closing bug.
<rdar://problem/54727835>