Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2938763002:
Keep previous size in Directory QuickView. (Closed)
DescriptionKeep previous size in Directory QuickView.
In QuickView of a directory, size field was flickering becuase every
time the directory is updated, the size field was hidden until its size
is retrieved from fileManagerPrivate.getDirectorySize.
Now it show previous directory size until the new size is retrieved if
the selected directory is not changed.
BUG=731484, 733143
TEST=manual
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2938763002
Cr-Commit-Position: refs/heads/master@{#479333}
Committed: https://chromium.googlesource.com/chromium/src/+/f34801c28ded7c524030c5fe565e9ac420c4ce28
Patch Set 1 #
Total comments: 3
Patch Set 2 : Resolve review comments. #
Messages
Total messages: 22 (14 generated)
Description was changed from ========== Keep previous size in Directory QuickView. In QuickView of a directory, size field was flickering becuase every time the directory is updated, the size field was hidden until its size is retrieved from fileManagerPrivate.getDirectorySize. Now it show previous directory size until the new size is retrieved if the selected directory is not changed. BUG=731484 TEST=manual ========== to ========== Keep previous size in Directory QuickView. In QuickView of a directory, size field was flickering becuase every time the directory is updated, the size field was hidden until its size is retrieved from fileManagerPrivate.getDirectorySize. Now it show previous directory size until the new size is retrieved if the selected directory is not changed. BUG=731484 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.
tetsui@chromium.org changed reviewers: + fukino@chromium.org, oka@chromium.org
fukino@: PTAL. Thanks!
https://codereview.chromium.org/2938763002/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/js/metadata_box_controller.js (right): https://codereview.chromium.org/2938763002/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/js/metadata_box_controller.js:206: chrome.fileManagerPrivate.getDirectorySize(entry, function(size) { FYI: This indent change is done by "git cl format --js ui/file_manager"
https://codereview.chromium.org/2938763002/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/js/metadata_box_controller.js (right): https://codereview.chromium.org/2938763002/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/js/metadata_box_controller.js:57: this.previousDirectorySize_ = -1; We should stop the flicker, but I think we still need to update the folder size when the content keep changing. Maybe the following approach works? * When the selected entry is not changed, we don't clear metadataBox_.isSizeLoading and metadataBox_.size. Instead, we update metadataBox_.size when we get the new size. Following is a nice-to-have improvement. It'd be great if you can file a bug. * To avoid excessive call to chrome.fileManagerPrivate.getDirectorySize(), we ignore setDirectorySize_() when we wait the callback for chrome.fileManagerPrivate.getDirectorySize. Note that the final request to getDirectorySize() should not be ignored to show the final folder size.
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 ========== Keep previous size in Directory QuickView. In QuickView of a directory, size field was flickering becuase every time the directory is updated, the size field was hidden until its size is retrieved from fileManagerPrivate.getDirectorySize. Now it show previous directory size until the new size is retrieved if the selected directory is not changed. BUG=731484 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Keep previous size in Directory QuickView. In QuickView of a directory, size field was flickering becuase every time the directory is updated, the size field was hidden until its size is retrieved from fileManagerPrivate.getDirectorySize. Now it show previous directory size until the new size is retrieved if the selected directory is not changed. BUG=731484,733143 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
https://codereview.chromium.org/2938763002/diff/1/ui/file_manager/file_manage... File ui/file_manager/file_manager/foreground/js/metadata_box_controller.js (right): https://codereview.chromium.org/2938763002/diff/1/ui/file_manager/file_manage... ui/file_manager/file_manager/foreground/js/metadata_box_controller.js:57: this.previousDirectorySize_ = -1; On 2017/06/14 06:27:55, fukino wrote: > We should stop the flicker, but I think we still need to update the folder size > when the content keep changing. > > Maybe the following approach works? > * When the selected entry is not changed, we don't clear > metadataBox_.isSizeLoading and metadataBox_.size. Instead, we update > metadataBox_.size when we get the new size. > > Following is a nice-to-have improvement. It'd be great if you can file a bug. > * To avoid excessive call to chrome.fileManagerPrivate.getDirectorySize(), we > ignore setDirectorySize_() when we wait the callback for > chrome.fileManagerPrivate.getDirectorySize. Note that the final request to > getDirectorySize() should not be ignored to show the final folder size. Implemented the suggested approach and created bug. http://crbug.com/733143 Thanks!
lgtm. Thanks!
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": 1497431292808450,
"parent_rev": "2c8d6871f9de55fe8fdc606efd101c39792a18a2", "commit_rev":
"f34801c28ded7c524030c5fe565e9ac420c4ce28"}
Message was sent while issue was closed.
Description was changed from ========== Keep previous size in Directory QuickView. In QuickView of a directory, size field was flickering becuase every time the directory is updated, the size field was hidden until its size is retrieved from fileManagerPrivate.getDirectorySize. Now it show previous directory size until the new size is retrieved if the selected directory is not changed. BUG=731484,733143 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Keep previous size in Directory QuickView. In QuickView of a directory, size field was flickering becuase every time the directory is updated, the size field was hidden until its size is retrieved from fileManagerPrivate.getDirectorySize. Now it show previous directory size until the new size is retrieved if the selected directory is not changed. BUG=731484,733143 TEST=manual CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2938763002 Cr-Commit-Position: refs/heads/master@{#479333} Committed: https://chromium.googlesource.com/chromium/src/+/f34801c28ded7c524030c5fe565e... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/f34801c28ded7c524030c5fe565e... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
