| Issue 59: | No links from side-by-side view back to the associated change | |
| Back to list |
Reported by Dave Bort <dbort@android.com> on Fri Jan 30 16:03:56 PST 2009 Source: JIRA GERRIT-59 Environment: Gerrit Code Review (v2.0-47-g9e727d3) Firefox 3.0 on Dapper x86-mixed - Go to a change like http://review.source.android.com/Gerrit#change,8718 - Click on "side-by-side" diff for a file, like http://review.source.android.com/Gerrit#patch,sidebyside,8718,1,core/find-jdk-tools-jar.sh Notice that on the resulting page, there are no links back to the change, so it's not obvious how to progress to the next file or to the "publish drafts" step. I also missed links to the next/previous file, and information providing the context of the change (branch, project, patch submitter, etc.)
Sep 24, 2009
#1
code-rev...@gtempaccount.com
Sep 24, 2009
Comment by Brad Larson <bklarson@gmail.com> on Tue Mar 24 20:37:47 PDT 2009 I got a chance to look into this a bit tonight. I'm mostly concerned with the 'back to change', 'previous file', and 'next file' links. I see two options for handling this: 1. Modify the Patch table to include a previous and next ID fields 2. Grab the PatchSetDetail from the DB when we load a patch page and loop through to find the prev/next keys 1 involves a DB schema change and lots more coding when a change is uploaded, but wouldn't require the extra DB hit(s). I'm favoring 2 right now because it seems more straight-forward. Shawn, would you be OK with that approach? Also, this is a really low priority for me right now... if somebody wants it sooner, feel free to grab the task from me.
Sep 24, 2009
Comment by Shawn Pearce <sop@google.com> on Wed Apr 08 09:14:07 PDT 2009 You're right, 1 will produce a more efficient implementation in the long run. But I was thinking that 2 might be the way to go here. In particular, we already have the entire PatchSetDetail loaded in the ChangeScreen that got us to the side by side view. So we could just pass that data down into the side by side view if we were created from it; and if not then we can use a background async RPC to load it. If we were really cute we could actually reuse the entire PatchTable UI widget from the ChangeScreen inside the side by side view, and pop it open into a PopupPanel when the user wants to jump to another file. This would give instant navigation in the client to nearly any file, and we'd only incur the database hit to load the entire list of patches once, when the change is first viewed, or once when the side by side view is started if the user direct linked to it.
Sep 24, 2009
Comment by Shawn Pearce <sop@google.com> on Fri May 15 18:41:07 PDT 2009 Partially handled by 'u' now being available as a keyboard shortcut up, but we still need a hyperlink.
Sep 24, 2009
Comment by Shawn Pearce <sop@google.com> on Sat May 30 11:32:20 PDT 2009 Fixed by https://review.source.android.com/10019
Sep 24, 2009
Update by Shawn Pearce <sop@google.com> on Sat May 30 11:32:20 PDT 2009 Assigned to Cedric Beust. Fixed in version 2.0.13.
Status:
Fixed
Sep 25, 2009
(No comment was entered for this change.)
Labels:
FixedIn-2.0.13
Oct 25, 2012
(No comment was entered for this change.)
Status:
Released
|
|
| ► Sign in to add a comment |