Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(139)

Issue 2540863004: Quick View: Support pdf and text preview. (Closed)

Created:
4 years ago by oka
Modified:
4 years ago
Reviewers:
fukino
CC:
chromium-reviews, yamaguchi+watch_chromium.org, oka+watch_chromium.org, rginda+watch_chromium.org, oshima+watch_chromium.org, fukino+watch_chromium.org, davemoore+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Quick View: Support pdf and text preview. This CL allows user to open files which are browsable on Chrome on Quick View. Browsable files include pdf and text. Screencast: https://bugs.chromium.org/p/chromium/issues/detail?id=640696#c11 BUG=642713, 640696 TEST=manually tested the following: - For pdf and text, preview with 24px of vertical margin appears. - For image, video and audio, preview with 15% of vertical margin appears as before. - For .docx, 'no preview available' is shown as expected, though the file type is 'document' which is the same as pdf. CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/885a00341b184c0daed52a7a7fb234f6f14638f6 Cr-Commit-Position: refs/heads/master@{#438744}

Patch Set 1 #

Patch Set 2 : Removed an irrelevant file. #

Patch Set 3 : Fix. #

Patch Set 4 : Rebased. #

Patch Set 5 : Made it work. #

Patch Set 6 : Fix nits. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -23 lines) Patch
M ui/file_manager/file_manager/foreground/elements/files_quick_view.css View 1 2 3 4 5 2 chunks +15 lines, -12 lines 0 comments Download
M ui/file_manager/file_manager/foreground/elements/files_quick_view.html View 1 2 3 4 5 3 chunks +9 lines, -6 lines 0 comments Download
M ui/file_manager/file_manager/foreground/elements/files_quick_view.js View 1 2 3 4 5 3 chunks +8 lines, -2 lines 0 comments Download
M ui/file_manager/file_manager/foreground/js/quick_view_controller.js View 1 2 3 4 3 chunks +25 lines, -3 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 18 (11 generated)
oka
PTAL.
4 years ago (2016-12-09 10:35:41 UTC) #6
fukino
lgtm assuming that the design got approval.
4 years ago (2016-12-09 11:15:47 UTC) #7
fukino
On 2016/12/09 11:15:47, fukino wrote: > lgtm assuming that the design got approval. BTW, please ...
4 years ago (2016-12-12 02:06:49 UTC) #9
oka
On 2016/12/12 02:06:49, fukino wrote: > On 2016/12/09 11:15:47, fukino wrote: > > lgtm assuming ...
4 years ago (2016-12-15 03:57:42 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2540863004/100001
4 years ago (2016-12-15 03:58:16 UTC) #13
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years ago (2016-12-15 04:39:19 UTC) #16
commit-bot: I haz the power
4 years ago (2016-12-15 04:41:29 UTC) #18
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/885a00341b184c0daed52a7a7fb234f6f14638f6
Cr-Commit-Position: refs/heads/master@{#438744}

Powered by Google App Engine
This is Rietveld 408576698