My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 258: Please make the image view work more like the text page viewers
7 people starred this issue and may be notified of changes. Back to list
Status:  Accepted
Owner:  cbe...@gmail.com
Cc:  stadlera...@gmail.com


Sign in to add a comment
 
Reported by code-rev...@gtempaccount.com, Sep 24, 2009
Reported by Andrew D. Stadler (Google) <stadler@android.com> on Fri Aug 14 13:34:37 PDT 2009
Source: JIRA GERRIT-259

Right now if you are reviewing a change and it has text files, you can use ']'
to navigate through the changes, file by file.

But if there's a PNG file included in the change you wind up in one of two
modes (neither being very useful):

a.  If you click the file name or "unified" you get

diff --git a/res/drawable/ic_list_drafts.png b/res/drawable/ic_list_drafts.png
			new file mode 100644
			index
0000000000000000000000000000000000000000..7ad323aef5fe6b8951ed3962b7659b204bd1cb4c
			Binary files /dev/null and b/res/drawable/ic_list_drafts.png differ

but at least you can navigate to the next file.

b.  If you click "new" you just see the icon.

I claim there's little value in the "b" case, you should always be taken to a
gerrit review page with header, navigation, etc.


Also:  Can you please draw a box around the image so the approx borders can be
seen?  Or perhaps even show it in 3..4 modes like "on white", "on black", "on
grey".  I know this seems silly but it's often a big part in evaluating an
image.
Sep 24, 2009
#1 code-rev...@gtempaccount.com
Comment by Shawn Pearce <sop@google.com> on Fri Aug 14 13:41:09 PDT 2009

"b" happens because its a binary file.

Imagine trying to review a radio firmware update.  You can't really display
that on the webpage.  So "new" link appears to allow you to download the file
to your system, where you can use other tools to examine it, tools that are
perhaps more suited to that type of file.

So "b" won't disappear.


"a" is Cedric's work, I'm moving this to him.  :-)
Sep 24, 2009
#2 code-rev...@gtempaccount.com
Update by Shawn Pearce <sop@google.com> on Fri Aug 14 13:41:22 PDT 2009

Assigned to Cedric Beust.
Sep 24, 2009
#3 code-rev...@gtempaccount.com
Comment by Andrew D. Stadler (Google) <stadler@android.com> on Fri Aug 14 13:51:41 PDT 2009

re "b".  The need (or ability) to download a file is completely orthogonal to
whether it's new or not.  For example, if you were *replacing* a radio image,
it should be possible to download new or old;  And this should be true for any
file type - even PNG, even text.

Also, a download link should be named "download".

For non-text files, wouldn't it be great if we always want to a page that
looks just like the others, with a left side and a right side, in all cases,
nice info like filename, file size, and one or two download links.  And in the
case of drawables, show the before and/or after images.
Sep 24, 2009
#4 sop+code@google.com
(No comment was entered for this change.)
Status: Accepted
Owner: ---
Sep 24, 2009
#5 sop+code@google.com
(No comment was entered for this change.)
Owner: cbeust
Cc: stad...@android.com
Jun 30, 2013
#6 krinklemail@gmail.com
See also issue 750.
Sign in to add a comment

Powered by Google Project Hosting