Page MenuHomePhabricator

Add DiffusionBranchListView for browsing branches
ClosedPublic

Authored by chad on Jun 10 2017, 4:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 4:56 AM
Unknown Object (File)
Wed, Nov 20, 9:36 AM
Unknown Object (File)
Sat, Nov 16, 1:00 PM
Unknown Object (File)
Tue, Nov 12, 5:32 AM
Unknown Object (File)
Mon, Nov 11, 6:36 AM
Unknown Object (File)
Sun, Nov 10, 4:12 PM
Unknown Object (File)
Sun, Nov 10, 1:04 AM
Unknown Object (File)
Sat, Nov 9, 4:13 AM
Subscribers

Details

Summary

Adds a new DiffusionBranchListView which replaces the BranchTable when browsing all branches in Diffusion. Has all the same capabilities, but is easier to read, adds a Compare button, and plays nicely on mobile. It does take up more space, but I think that's generally OK here since we expect our branches to not be heaping piles of intern revert branches.

Test Plan

Follow a few repositories with branches, like Phabricator and KDE's Krita. View layouts on mobile, tablet, desktop. Try out new compare button.

image.png (1×2 px, 214 KB)

Diff Detail

Repository
rP Phabricator
Branch
diff-branch-list (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 17491
Build 23457: Run Core Tests
Build 23456: arc lint + arc unit

Event Timeline

blerg, how do I re-parent this commit to be on master and not on arcpatch-D12345? google is not really helping so much

I'd do this:

$ git checkout master
$ git cherry-pick 00a6a0a06927

(Assuming you want that exact commit; Otherwise, find the right hash, or specify a few commits in cherry-pick in the right order)

I think I would expect the "Compare" button to pop this dialog instead:

Compare: [branch you just clicked]
     To: [default branch in the repository]

...where both inputs are text entry forms (so you can type a commit hash instead). It is not intuitive to me that the "branches" UI would behave in a way that is stateful with respect to the current branch.

I think the autoclose stuff could be moved into "Manage" and removed from this UI (e.g., show a simple table under ManageBranches of each branch and its autoclose/track status).

I think the "current" marker is not really useful/meaningful, at least today. There is very little UI for statefully switching branches, and this isn't a concept we emphasize. A better marker might be "Default".

Your test plan does not appear to include Mercurial repositories (unless Krita has an hg repo?) but should.

This revision is now accepted and ready to land.Jun 12 2017, 3:17 PM

Is there a way to get the default branch? The code was ($branch->getShortName() == $current_branch) so it gave me the impression it was dependent on the branch you were viewing. Personally I'd think comparing against master is the most common? So one click to that comparison page, then a second click to specify a hash? Or is there a workflow I'm not imagining? I think this was @avivey 's work.

$repository->getDefaultBranch() has the default branch for the repository.

This is usually (but not always) "master". This will also usually (but not always) be the current branch.

Here's a case where the two are different, although it's a little bit made up:

  • You navigate to /rXYZabc, which you've, for example, used git show stable from the CLI to determine is the head of stable.
  • This commit is an ancestor of the branch heads of stable and release-20170103, so the "Branches" field in the UI shows those branches.
  • You click stable, then you click some link to end up on this UI with "stable" selected as the current branch (I believe this is impossible today, and that no link in the product leads to this UI with any branch other than the default branch selected as the current branch).
  • You want to compare stable to master, but there's no button next to stable, and the button next to master compares master to stable (the opposite of what you want).

In the UI I propose, the dialog is default-filled with the branch names. For example, in the workflow above, clicking "Compare" would show this dialog:

Compare: [stable]
     To: [master]

This is one more click than automatically comparing to the default branch (you must click "Compare" in the dialog to accept the defaults). However, it lets you see what "Compare" will do, and change the targets of the operation (e.g., from a branch head to a particular commit hash) if you have a different range in mind.

This is the same UI we currently implement in the browse view today, so if we change it here we should also change it there:

Screen Shot 2017-06-12 at 9.11.45 AM.png (1×1 px, 349 KB)

(As a general note, I've intentionally buried the "Compare" view a bit because I think it still needs significant work, although we haven't seen negative feedback about it that I can recall so it may be in better shape than I thought.)

Well if it's UI work, toss it into the stack. I was planning a pass there as well.

The major thing is that the compare view currently shows a list of commits present on A but not B, but does not show the actual diff of source changes.

On GitHub, the UI looks like this:

Screen Shot 2017-06-12 at 9.19.28 AM.png (1×1 px, 351 KB)

We implement a similar UI to the "Commits" tab (not selected) but do not have an equivalent feature to the "Files changed" tab (selected).

I suspect this is probably outside the realm of "UI changes", although it's technically a UI change I guess?

haha, well UI changes should be == work I can do.

  • rebase onto master
  • update per comments
  • re-re-read feedback and make adjustments
This revision was automatically updated to reflect the committed changes.