evanchueng / gerrit

Automatically exported from code.google.com/p/gerrit
Apache License 2.0
0 stars 0 forks source link

Please make the image view work more like the text page viewers #258

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
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..7ad323aef5fe6b8951ed3962b7659b204bd1cb
4c
            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.

Original issue reported on code.google.com by code-rev...@gtempaccount.com on 24 Sep 2009 at 7:44

GoogleCodeExporter commented 9 years ago
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.  :-)

Original comment by code-rev...@gtempaccount.com on 24 Sep 2009 at 8:39

GoogleCodeExporter commented 9 years ago
Update by Shawn Pearce <sop@google.com> on Fri Aug 14 13:41:22 PDT 2009

Assigned to Cedric Beust.

Original comment by code-rev...@gtempaccount.com on 24 Sep 2009 at 8:39

GoogleCodeExporter commented 9 years ago
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.

Original comment by code-rev...@gtempaccount.com on 24 Sep 2009 at 8:39

GoogleCodeExporter commented 9 years ago

Original comment by sop+code@google.com on 24 Sep 2009 at 10:11

GoogleCodeExporter commented 9 years ago

Original comment by sop+code@google.com on 24 Sep 2009 at 10:54

GoogleCodeExporter commented 9 years ago
See also issue 750.

Original comment by krinklemail@gmail.com on 30 Jun 2013 at 5:47