Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1995783002:
DevTools: Add inertia to flamechart dragging. (Closed)
|
Created:
4 years, 7 months ago by alph Modified:
4 years, 7 months ago Reviewers:
caseq CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, sergeyv+blink_chromium.org, pfeldman, kozyatinskiy+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: Add inertia to flamechart dragging.
Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab
Cr-Commit-Position: refs/heads/master@{#394666}
Committed: https://crrev.com/3a65384e009bcc8fe0e05bccb963f60ab6330ead
Cr-Commit-Position: refs/heads/master@{#394884}
Patch Set 1 #
Total comments: 10
Patch Set 2 : fixing nits #Patch Set 3 : fix click - call dragEnd immediately if velocity is already below the threshold. #
Messages
Total messages: 19 (9 generated)
alph@chromium.org changed reviewers: + caseq@chromium.org
lgtm https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right): https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:220: /** nit: empty line above https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:233: /** ditto https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:238: var now = Date.now(); s/Date.now/performance.now/ everywhere https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:250: function animationStep() ditto https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:263: var k = Math.pow(1 / (1 + friction), duration / 1000); can we just compute velocity base on the time since start of fling, not between two steps -- the math would be the same, but I think this is going to be a bit easier to read.
https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js (right): https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:220: /** On 2016/05/19 00:38:36, caseq wrote: > nit: empty line above Done. https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:233: /** On 2016/05/19 00:38:36, caseq wrote: > ditto Done. https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:238: var now = Date.now(); On 2016/05/19 00:38:36, caseq wrote: > s/Date.now/performance.now/ everywhere Done. https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:250: function animationStep() On 2016/05/19 00:38:36, caseq wrote: > ditto Done. https://codereview.chromium.org/1995783002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/UIUtils.js:263: var k = Math.pow(1 / (1 + friction), duration / 1000); On 2016/05/19 00:38:36, caseq wrote: > can we just compute velocity base on the time since start of fling, not between > two steps -- the math would be the same, but I think this is going to be a bit > easier to read. That would complicate code a bit. I'd leave it as is.
The CQ bit was checked by alph@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org Link to the patchset: https://codereview.chromium.org/1995783002/#ps20001 (title: "fixing nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995783002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995783002/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== DevTools: Add inertia to flamechart dragging. ========== to ========== DevTools: Add inertia to flamechart dragging. Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666}
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1995123002/ by alph@chromium.org. The reason for reverting is: Broke item selection on flamechart.
Message was sent while issue was closed.
Description was changed from ========== DevTools: Add inertia to flamechart dragging. Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666} ========== to ========== DevTools: Add inertia to flamechart dragging. Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666} ==========
The CQ bit was checked by alph@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org Link to the patchset: https://codereview.chromium.org/1995783002/#ps40001 (title: "fix click - call dragEnd immediately if velocity is already below the threshold.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995783002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995783002/40001
Message was sent while issue was closed.
Description was changed from ========== DevTools: Add inertia to flamechart dragging. Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666} ========== to ========== DevTools: Add inertia to flamechart dragging. Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666} ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== DevTools: Add inertia to flamechart dragging. Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666} ========== to ========== DevTools: Add inertia to flamechart dragging. Committed: https://crrev.com/9d73e469f31c85095dd8469dfe72971721310fab Cr-Commit-Position: refs/heads/master@{#394666} Committed: https://crrev.com/3a65384e009bcc8fe0e05bccb963f60ab6330ead Cr-Commit-Position: refs/heads/master@{#394884} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/3a65384e009bcc8fe0e05bccb963f60ab6330ead Cr-Commit-Position: refs/heads/master@{#394884} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
