Ref T3718. Ref T3644. Ref T3092. Switches from the Releeph UI elements to standard ones. I'll attach some screenshots.
Also fixes CSRF against the request action endpoint.
Differential D8771
Use standard UI elements to render pull requests in Releeph epriestley on Apr 13 2014, 5:55 PM. Authored by Tags None Referenced Files
Details
Ref T3718. Ref T3644. Ref T3092. Switches from the Releeph UI elements to standard ones. I'll attach some screenshots. Also fixes CSRF against the request action endpoint.
Diff Detail
Event TimelineComment Actions Here's a summary of the changes:
Comment Actions I'd like to avoid another insurrection, so please give us feedback on these UI changes if you think they're significantly disruptive to your workflows. (The new element is slightly larger overall than the old one was, mostly because the message is rendered using a large font size. Tentatively, I eventually intend to let each request include more than one commit (i.e., so users can request an entire branch, like on GitHub) and show the commits in a table with the messages summarized at the bottom of this UI. However, this may not happen for a while.) Comment Actions I think we should drop the multi-column layout: Currently, pull requests in Releeph always request one commit, and the UI roughly puts information about the requestor in one column and information about the commit in the other column. This is reasonable in SVN and some Git workflows, but makes less sense in branch-based Git workflows, and isn't what users coming from GitHub will expect (on GitHub, pull requests are essentially always for "all commits on a branch", not "specific commit X"). I think that sticking with this model will reduce the usefulness of the tool in general and create a lot of user confusion. Assuming we move away from this model, this UI won't make sense anyway. Beyond that, no other objects use multiple columns, and none really have a compelling argument for it. Since nothing else does this, none of the infrastructure has support for it. For example, we have a drag-and-drop control for reordering fields in configuration, but it doesn't support choosing which column they will appear on. Overall, supporting multiple columns requires a large amount of custom code (this diff removes almost a thousand lines in net, and more big chunks of code can be dropped beyond that) for what I think is a pretty questionable gain. At least for me, these new UIs and supporting interactions look much better and are easier to read. Of course, they're also different... Comment Actions We got 112 clicks from Facebook employees and no complaints, so I'm going to move this forward. Thanks for taking a look! For future readers: if and when there are complaints down the line, we're happy to make adjustments to this interface. We can be most effective in addressing specific issues (like: field X used to do Y, but doesn't any more; I'm colorblind and Z is harder to read than it used to be; field Q isn't very important, but now has a lot more space dedicated to it; etc etc). We probably can't be very effective in addressing "put it back like it was and never change anything" feedback, because this kind of feedback conflicts directly with all other goals, and we made every reasonable effort we could to collect this kind of "general dissatisfaction" feedback here, before making this change. |