Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2932803002:
Stop updating context menu task items when not changed. (Closed)
DescriptionStop updating context menu task items when not changed.
During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED,
existing task items in context menu were overwritten by a temporary
item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly,
which leads to flickering context menu during copy.
If selection is not changed, we should not clear the task items in the
context menu.
BUG=731483
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2932803002
Cr-Commit-Position: refs/heads/master@{#478239}
Committed: https://chromium.googlesource.com/chromium/src/+/f8c224c31c133a07f58a8d922ec15aa730c06cde
Patch Set 1 #
Total comments: 1
Patch Set 2 : Compare selected entries instead of removing temporary item. #Messages
Total messages: 23 (16 generated)
Description was changed from ========== Remove a temporary context menu task item. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. In theory, this change may show unmatching task items for short period of time. But this is less likely to happen and it simplifies the code. BUG=731483 TEST=manual ========== to ========== Remove a temporary context menu task item. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. In theory, this change may show unmatching task items for short period of time. But this is less likely to happen and it simplifies the code. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by tetsui@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Remove a temporary context menu task item. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. In theory, this change may show unmatching task items for short period of time. But this is less likely to happen and it simplifies the code. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Remove a temporary context menu task item. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. In theory, this change may show unmatching task items for short period of time. But this is unlikely observable from users compared to the flickering, and it simplifies the code. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
tetsui@chromium.org changed reviewers: + fukino@chromium.org, oka@chromium.org
Please take a look. Thank you!
https://codereview.chromium.org/2932803002/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/js/task_controller.js (left): https://codereview.chromium.org/2932803002/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/js/task_controller.js:261: * Handles change of selection and clears context menu. updateTasks_() can take long time to update the context menu. If we don't clear the context menu, the user may see the wrong context menu which is for previously-selected files. (It is not necessarily a flash ,depending on the time taken by updateTasks_) I'm wondering why the context menu is updated even when the selected entries are not changed. Maybe we should check if the entries for the current context menu is changed or not?
The CQ bit was checked by tetsui@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Remove a temporary context menu task item. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. In theory, this change may show unmatching task items for short period of time. But this is unlikely observable from users compared to the flickering, and it simplifies the code. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Do not update context menu task items when not changed. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. If selection is not changed, we should not clear the task items in the context menu. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Do not update context menu task items when not changed. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. If selection is not changed, we should not clear the task items in the context menu. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Stop updating context menu task items when not changed. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. If selection is not changed, we should not clear the task items in the context menu. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
On 2017/06/09 06:24:51, fukino wrote: > https://codereview.chromium.org/2932803002/diff/1/ui/file_manager/file_manage... > File ui/file_manager/file_manager/foreground/js/task_controller.js (left): > > https://codereview.chromium.org/2932803002/diff/1/ui/file_manager/file_manage... > ui/file_manager/file_manager/foreground/js/task_controller.js:261: * Handles > change of selection and clears context menu. > updateTasks_() can take long time to update the context menu. > If we don't clear the context menu, the user may see the wrong context menu > which is for previously-selected files. (It is not necessarily a flash > ,depending on the time taken by updateTasks_) > > I'm wondering why the context menu is updated even when the selected entries are > not changed. > Maybe we should check if the entries for the current context menu is changed or > not? Done.
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tetsui@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497001302238990,
"parent_rev": "0ae54aecb40296d0f2304176ffec747e01bc5643", "commit_rev":
"f8c224c31c133a07f58a8d922ec15aa730c06cde"}
Message was sent while issue was closed.
Description was changed from ========== Stop updating context menu task items when not changed. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. If selection is not changed, we should not clear the task items in the context menu. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Stop updating context menu task items when not changed. During FileSelectionHandler.EventType.CHANGE to CHANGE_THROTTLED, existing task items in context menu were overwritten by a temporary item. File copy operation fires CHANGE and CHANGE_THROTTLED repeatedly, which leads to flickering context menu during copy. If selection is not changed, we should not clear the task items in the context menu. BUG=731483 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2932803002 Cr-Commit-Position: refs/heads/master@{#478239} Committed: https://chromium.googlesource.com/chromium/src/+/f8c224c31c133a07f58a8d922ec1... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/f8c224c31c133a07f58a8d922ec1...
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/2973003002/ by yamaguchi@chromium.org. The reason for reverting is: We'd like to revert 2833413003 because it caused regression crbug.com/738803. This is change depends on 2833413003.. |
