RESOLVED FIXED 52226
allow sidebysideifying individual files in the code review tool 
https://bugs.webkit.org/show_bug.cgi?id=52226
Summary allow sidebysideifying individual files in the code review tool
Ojan Vafai
Reported 2011-01-11 11:11:25 PST
allow sidebysideifying individual files in the code review tool
Attachments
Patch (2.84 KB, patch)
2011-01-11 11:12 PST, Ojan Vafai 		abarth: review+
abarth: commit-queue-
DetailsFormatted DiffDiff
screenshot (123.07 KB, image/png)
2011-01-11 11:12 PST, Ojan Vafai 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2011-01-11 11:12:07 PST
Created attachment 78557 [details] Patch
Ojan Vafai
Comment 2 2011-01-11 11:12:31 PST
Created attachment 78558 [details] screenshot
Adam Barth
Comment 3 2011-01-11 12:06:17 PST
The fonts don't seem to match the existing side-by-side links. Also, maybe we should have one link that changes text based on the current state? I'm worried about overloading the reviewtool with lots of UI.
Ojan Vafai
Comment 4 2011-01-11 12:31:40 PST
(In reply to comment #3) > The fonts don't seem to match the existing side-by-side links. It's just inheriting the font. I'll fix that. > Also, maybe we should have one link that changes text based on the current state? I'm worried about overloading the reviewtool with lots of UI. I'm sympathetic to this. I think this is valuable though. We can certainly remove it later if people find it to be too much. It's just a few lines of code. I don't have plans to add any other UI after this. The only tasks I have planned for the review tool: -Make side-by-side diffs more minimal by actually putting removed lines to the left of added lines. -Store whether you want side-by-side diffs in the cookie so you can always load the one you prefer by default.
Adam Barth
Comment 5 2011-01-11 14:18:20 PST
Can we hide the link that's not meaningful (e.g., side-by-side when we're already in side-by-side mode)? Also, I would vertically center the link in the H1.
Adam Barth
Comment 6 2011-01-11 14:52:36 PST
Comment on attachment 78557 [details] Patch Lets land this with the cosmetic changes discussed above. It would be nice to add the link-hiding in a followup patch.
Ojan Vafai
Comment 7 2011-01-11 15:16:41 PST
Committed r75559: <http://trac.webkit.org/changeset/75559>
Note You need to log in before you can comment on or make changes to this bug.