Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
440853002:
DevTools: NetworkPanel: get rid of stripes gradient. (Closed)
|
Created:
6 years, 4 months ago by eustas Modified:
6 years, 4 months ago CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, malch+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, sergeyv+blink_chromium.org, aandrey+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@master Project:
blink Visibility:
Public. |
DescriptionDevTools: NetworkPanel: get rid of stripes gradient.
Rendering gradient consumes considerable amount of time.
BUG=316092
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180449
Patch Set 1 #Patch Set 2 : #
Total comments: 1
Patch Set 3 : #
Total comments: 2
Patch Set 4 : #
Total comments: 2
Patch Set 5 : #
Messages
Total messages: 23 (0 generated)
Edit the description and set BUG=316092? :)
On 2014/08/05 11:59:52, PhistucK wrote: > Edit the description and set BUG=316092? :) Surely =)
Ping
https://chromiumcodereview.appspot.com/440853002/diff/20001/Source/devtools/f... File Source/devtools/front_end/networkLogView.css (right): https://chromiumcodereview.appspot.com/440853002/diff/20001/Source/devtools/f... Source/devtools/front_end/networkLogView.css:53: background-image: linear-gradient(to bottom, white, white 50%, #eee 50%, #eee); Can we set the background color of the rows explicitly now that we use a viewport?
On 2014/08/14 08:20:25, vsevik wrote: > https://chromiumcodereview.appspot.com/440853002/diff/20001/Source/devtools/f... > File Source/devtools/front_end/networkLogView.css (right): > > https://chromiumcodereview.appspot.com/440853002/diff/20001/Source/devtools/f... > Source/devtools/front_end/networkLogView.css:53: background-image: > linear-gradient(to bottom, white, white 50%, #eee 50%, #eee); > Can we set the background color of the rows explicitly now that we use a > viewport? Probably yes.
PTAL
could you please attach a screenshot of the UI? https://codereview.chromium.org/440853002/diff/40001/Source/devtools/front_en... File Source/devtools/front_end/ui/ViewportDataGrid.js (right): https://codereview.chromium.org/440853002/diff/40001/Source/devtools/front_en... Source/devtools/front_end/ui/ViewportDataGrid.js:111: var oddFirst = ((start & 1) === 0); Though I love bit operations, I'd say that it is less clear for the average reader than simple "% 2 === 0".
UI didn't change at all. https://codereview.chromium.org/440853002/diff/40001/Source/devtools/front_en... File Source/devtools/front_end/ui/ViewportDataGrid.js (right): https://codereview.chromium.org/440853002/diff/40001/Source/devtools/front_en... Source/devtools/front_end/ui/ViewportDataGrid.js:111: var oddFirst = ((start & 1) === 0); On 2014/08/15 15:58:33, lushnikov wrote: > Though I love bit operations, I'd say that it is less clear for the average > reader than simple "% 2 === 0". Done.
lgtm https://codereview.chromium.org/440853002/diff/60001/Source/devtools/front_en... File Source/devtools/front_end/ui/ViewportDataGrid.js (right): https://codereview.chromium.org/440853002/diff/60001/Source/devtools/front_en... Source/devtools/front_end/ui/ViewportDataGrid.js:147: element.classList.toggle("odd", odd); Consider doing something like the code below to reduce the amount of code for this feature: element.classList.toggle("odd", (start + i) % 2 === 0);
https://codereview.chromium.org/440853002/diff/60001/Source/devtools/front_en... File Source/devtools/front_end/ui/ViewportDataGrid.js (right): https://codereview.chromium.org/440853002/diff/60001/Source/devtools/front_en... Source/devtools/front_end/ui/ViewportDataGrid.js:147: element.classList.toggle("odd", odd); On 2014/08/18 05:46:55, vsevik wrote: > Consider doing something like the code below to reduce the amount of code for > this feature: > element.classList.toggle("odd", (start + i) % 2 === 0); Done.
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/440853002/80001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_compile_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/bu...) win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23157)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23169)
The CQ bit was checked by eustas@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eustas@chromium.org/440853002/80001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/23196)
Message was sent while issue was closed.
Committed patchset #5 (80001) as 180449 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
