HomePhabricator

Help PROFESSIONAL SOFTWARE ENGINEERS copy text to their clipboard

Description

Help PROFESSIONAL SOFTWARE ENGINEERS copy text to their clipboard

Summary: Ref T12780. I'd like 18,000 GitHub stars now please thank you

Test Plan: this feature is awful

Reviewers: chad

Reviewed By: chad

Subscribers: cspeckmim

Maniphest Tasks: T12780

Differential Revision: https://secure.phabricator.com/D18053

Details

Provenance
epriestleyAuthored on May 31 2017, 3:00 AM
epriestleyPushed on May 31 2017, 5:13 PM
Reviewer
chad
Differential Revision
D18053: Help PROFESSIONAL SOFTWARE ENGINEERS copy text to their clipboard
Parents
rPf8581f687c9b: Restrict green button to buttons
Branches
Unknown
Tags
Unknown
Tasks
T12780: Icons look bad in HistoryTableView
Build Status
Buildable 17341
Build 23239: Run Core Tests

Event Timeline

If it helps, we copy the full SHA with the button, which isn't exposed elsewhere on the object item.

Offhand, I can't think of a situation where I want the full hash instead of the shortened one. I copy/paste from this UI (if I ever do? maybe not? But the reasons I copy/paste hashes...) for either:

  • copy/pasting to the CLI, where either works fine and the short one is a little less cumbersome; or
  • copy/pasting to Remarkup, where the shorter one is preferable / more readable.

I think our rationale here is just "copy GitHub" and "copy GitLab, which copied GitHub". We don't really need design/product/use case justification -- it's fine for our goal to just be "feel like GitHub, which people like and are familiar with" even if none of the features are actually useful for software development.

I even could imagine that GitHub may have this UI because their commit hash is a (redundant) link, so maybe at some point someone said "it should be easier to copy commit hashes, it's kind of hard because they're a link", and some intern said "oh, I'll add a 'copy' button next to the commit hash to make that easier, obviously that's the right fix" without really thinking about the product. Then GitLab copied GitHub and now we've copied GitLab.

My goal is to use Diffusion, myself. I don't use it because I can do the same tasks in about half the time one GitHub. I agree though things like copy to clipboard are exceedingly minor points of polish.

The current design of Diffusion is largely because I built it to be usable for me: most things I do are much faster/easier in Diffusion than GitHub.

For example, I often use the file typeahead, which is why it's prominent in Diffusion.

If our goal here is "copy GitHub", we should just copy GitHub. That feels like what we're doing.

If our goal is "make Diffusion better for tasks X, Y, Z", are those tasks which can be written down, e.g. as feature requests? Offhand, I can't come up with a single thing I do which this UI is better at. It's prettier; it feels a lot like GitHub. Those aren't necessarily bad things. But I don't think it's more useful for any task I actually accomplish during the course of work.

There are a number of tasks I perform daily on GitHub over Diffusion, and those are the first cases I looked at. What do I go there for, why do I prefer it, and what problems do their UI decisions solve that ours do not. Some of these are simple and obvious, such as splitting up dates and making the title of the commit more readable. There are only so many ways to solve those problems, so it's just going to be similar. I can't really fault GitHub likewise for using our accept/request changes/comment model because it's been well honed and proven over time. I had hoped we could use these obvious designs as a base and layer on Phabricator specific needs and integrations, like the attempt at designing out an entire "commit pipeline" on the object, but even if it looked nice in my head and was potentially useful in a narrow set of cases, feedback suggested these things weren't very useful and improving the individual commit pages would be a better use of time. Some of this feedback is only going to happen outside of Photoshop land and in real use. The second set of use cases is pre-anticipated Diffusion after T5000, making branches easier to find, navigation, action bars, etc, and all floating in my head.

Diffusion overall is a large and unwieldy product, for me especially, I think I've never "formally" redesigned it top to bottom. I have about 20 ideas in my head and an overall vision and goal, but breaking that out into small, reviewable pieces along with discussion and changes is going to take time and some of the ideas aren't going to stick and some of the ideas will be a lot like GitHub, but overall it's not my intention to "copy GitHub" but just vastly improve the usability of Diffusion overall (some of which GitHub has already solved).

So, for example, I don't think I ever use commit dates in log views. I can't even think of anything that commit dates are useful for. It isn't obvious to me why splitting up dates is "simple" or "obvious" or useful at all, or what root problem it's solving, or why it's the best solution to that problem. I don't remember exactly, but suspect I added a "date" column to the commit table originally almost purely for aesthetic/consistency reasons, and to echo the dates in the browse view (which are -- very rarely -- slightly useful). It's particularly not simple/obvious/evident why putting a date line on each element and a date divider in the header is motivated by any concrete problem. They seem useless to me in GitHub and GitLab, too.

I'm fine with this, I just want to be clear that I do not understand any of the root problems this UI is solving, other than "make it look like GitHub" and "make it prettier". If a user filed a task tomorrow that said "copy should copy the short hash, not the full hash", I would just say "ask chad, I don't know why this works like it does" or "the full hash is what GitHub does so that's the best behavior, our goal is to be similar to GitHub". In other areas of the product I can generally justify the design from use cases and problems, and propose design or behavioral changes which don't compromise problems we've solved. If you're rebuilding this design to solve problems that you can't document/communicate, I won't be able to maintain it without possibly breaking the things you built it to solve. Not necessarily a problem, but I worry we may be in a rough spot in T5000 if neither of us have a complete understanding of what goals the product is serving.

I use date headers because they help orient my reading of history. When I go in it's usually for some commit I have a vague idea about the author and the timeframe, but not a title I can search on. So the three most important things for me in a history view is when, who, and what.

There isn't any great design decision on including a copy to clipboard button, I added it since we decided to remove the pipeline stuff. And since I was adding it, it seemed reasonable since there was already a short hash easily copyable, that a long hash on the button possibly solved a different problem vs. just being a straight duplicate.

I'd probably still prefer lint/audit/build status there though.

Why do you search for commits in the first place?

For example, this morning, I searched for 84742a94db1301a80534fa0d90abb3464cb57aae and d6a620be4577941092965429e4998e98f9cc469a to link them to T12787. I knew I had authored the first one some time in the last month, and you had authored the second one a little before that.

I used git log [--oneline] src/applications/maniphest to find them, since this is much faster than any web UI could ever reasonably be (maybe arc history src/applications/maniphest opening a browser window could conceivably be faster).

If I hadn't, searching for "feed points" finds the first one as the first hit, and "maniphest modular" finds the second one as the second hit (or the first hit for "maniphest modular transactions"). But typing that stuff and swapping my search to commits is slower than using git log from the CLI.

Using the web UI to find these commits in either GitHub or Diffusion takes about the same amount of time for me, and that time is overwhemingly spent in command-f + reading results+ paging or navigating to src/applications/maniphest, not browsing the results. The author information isn't useful at all since I'm command-f'ing for "points" or "transactions" by the time I make it to the page, and there's no way to filter by author. (Even if there was, git log --author chad ... would be far faster.)

I've lost track of what this conversation is at it's core about. I presumed I was following all your feedback from T12780. I don't really know why we added clipboard functionality if we weren't going to try it out.

Like, T12780 reads to me like "why don't you just copy GitHub, since that's what 95% of people want", but maybe I misunderstood tone?

Maybe another take on this is that it's essentially inconceivable to me that a professional software engineer finds the "copy" button convenient or useful. I probably copy-paste hundreds (thousands?) of times a day and I assume everyone else does too. This button is slower for me than double-click-to-select + command-c to copy, since I already have muscle memory for that and have to interrupt it and use this button instead.

I can imagine this button is useful if:

  1. You're on mobile. Selecting text on mobile, at least on an iPhone SE for me, is a huge pain. This button is super great on mobile. If this was a mobile-only feature, I'd be 100% sold on it. Even if my bitter resentment of this feature eventually wins, I'd support keeping it on mobile if we have space for it in the UI.
  2. You're ONLY use the mouse, and copy text by going to Edit โ†’ Copy.
    • Or some variant of this: you can't touch type, aren't very familiar with โŒ˜+C, etc.
  3. You have an accessibility issue which makes it difficult to double-click, point precisely, etc., and a single action (instead of a series of actions) or a keyboard-focusable button (instead of text which you can't focus) is easier for you to execute.

This is on desktop, so presumably we aren't looking at (1).

I assume there are zero software professionals in bucket (2), since I can't imagine someone who does anything technical for a living not using keyboard commands as an ingrained part of their workflow.

If we have users in bucket (3), maybe we could solve this for them more generally: for example, add a copyable sigil to every monogram/hash that makes it easy to hit with the keyboard focus or easy to copy with a single click.


But maybe my assumption on (2) here is not very good and a large number of users who aren't fluent in navigating software UIs?

Like, I also don't understand the motivation for T10469 at all. Browser tabs already do a much, much better job of it for everything I do. Any implementation we could ever pursue would be strictly worse for me than browser tabs.


Generally, I worry we may have some problems which look like this:

TaskWith Diffusion UIWith GitHub UIWith CLI
Find a commit by rough author/date45 seconds30 seconds4 seconds

With use cases like this, changes to move us toward the GitHub UI look great until you look at the third column.

The core of this conversation to me is about the motivation for this feature.

I think the only motivation for this feature is "copy GitHub".

If that's the justification for this feature, I can maintain this code. When someone files a task next week saying "it should copy the short hash, not the long hash, the long hash isn't useful", I know how to answer that task: no, that's not what GitHub does, and we're copying GitHub.

Your comment introduced a new motivation, "this provides a way to access the long hash".

If that's the justification for this feature, I can't maintain this code. When someone files a new task next week saying "it should copy the short hash, not the long hash, the long hash isn't useful", I don't know how to answer that. I don't have any reason to access the long hash. I don't know why it's important to access the long hash. I'd just have to say "I don't know why it works like this and can't think up any reasons for it, ask chad".

Sorry, I thought I was clear I could care less about this copy button.

Overall, I prefer reading a diff over a commit message.

Likewise, if you filed a task like "it's harder for me to find commits by rough author/date in Diffusion than in GitHub", I don't think I'd end up with GitHub-style date dividers. Instead, I'd say "I use git log to accomplish that task, what are the barriers to doing that in your case?" since you I think you can accomplish this task something like 10x faster if you use git log than with any web UI and that tailoring the web UI to solving that problem means we're preventing it from solving other problems that it could be much better at than the CLI.

I dunno, I just tried that CLI version and it's really hard for me to read / parse.

https://github.com/phacility/phabricator/commits/master appears when I type gi in my browser, so it's not like 30 seconds to get to

So your workflow for finding, say, d6a620be45, knowing only that the author is chad, the commit was in the last month or so, and that it's about modular transactions in Maniphest is:

My flow from that page is:

  • โŒ˜+F for "chad", see that searching by author is useless since you commit a lot.
  • โŒ˜+F for "modular", โŒ˜+G to cycle through them.
  • Next Page, โŒ˜+G to cycle.
  • Next Page, โŒ˜+G to cycle (It's on page 5.)
  • Give up, go back to the repository index, go to src โ†’ applications โ†’ maniphest, click "History", โŒ˜+F for "modular".
  • Find the commit.
  • Delete my gi alias because that took way longer than git log.

The longer it takes to explain to me a more optimal way, the more it really only proves my point.

It's tiring and exhausting to have "there is no point to good design" conversations.

Okay, we don't need to keep talking about this. I'll support whatever direction you want to take here as best I can.

I'm sorry, I'm just frustrated here. My goal is simply to make the page more readable, and have all the same information as the previous design. It feels like we're discussing why someone would use this page to begin with, which maybe isn't something I gave a lot of thought to. I'd like to understand what workflows that land here this design doesn't serve instead of maybe looking at why overall readability improvements needed to be made. For me at least, having things like faces, commits broken up by headers, larger fonts, make the page more legible and easier to parse vs a dense table.

Here are some things which I think this new UI isn't as good at as the old UI. These are all things I do personally, at least occasionally. I don't know if they're common or not, how much other users do them, whether we should be designing for them, etc.

Find Revisions for Commits: Sometimes I know that a commit was made recently, and it feels a little faster to find the revision for it by using web UI history than the CLI. My web workflow is:

  • Go to the repository page.
  • Scroll down, finding the commit visually or with โŒ˜+F.
  • Click or copy the Dxxxx link next to the commit.

The equivalent from the CLI is something like git log --oneline + git show <commit> + copy/paste the URL, but this feels a little slower if I already know that the commit was made recently since I have to run more commands and do more copy/pasting.

The equivalent from the web UI without this information is to just click to the detail page, then pick up the revision from there. This is only very slightly slower, maybe an extra second. I do this fairly rarely.

Another approach might be to build arc log, since I could probably do this even faster from the CLI if there was a version if we had a command like this:

$ arc log
abcdef0123 D1234 epriestley blah blah blah
...

I could do this today by wrapping git but parsing the commit messages is a bit of a pain and I don't do this often enough for it to feel very relevant. I expect to build arc log at some point, largely motivated by making this transition from commit to revision a little more convenient.

Find Origin of Build Failures: If master is not building, seeing build status in the history view is helpful to identify the first commit which broke the build, so I know what needs to be fixed. My web workflow is:

  • Go to the repository page.
  • Scroll down, finding the first commit with a red "X".
  • (If the whole thing is red, I might need to click "View All History" to dig deeper.)
  • That's what broke the build.

The equivalent from the CLI is so much work that it's probably not realistic (individually copy-paste each commit into the web UI, then visually inspect the build status). This task can also be accomplished via Harbormaster, but there isn't a good way to query Harbormaster "by branch" or "by history" (T11763 is somewhat related) today. A hypothetical arc log could show build status to make this easier from the CLI.

The equivalent from the web UI without this information is to right-click each commit into a new tab, then cycle through them until I find the first one which isn't broken.

I do this very rarely today because we almost never break the build, and when we do it's almost always Celerity and only broken for one commit. However, I believe this becomes much more important as the size of the codebase (or the number of flaky unit tests you write) increases.

Above a certain scale and with a sufficiently undisciplined ruleset around tests, this may become useless again because, e.g., master will always be broken and you only care about which commit first broke a particular test. This "find which commit broke test X" UI will almost certainly need to live in Harbormaster (probably growing out of T11763) and could also serve this use case (find which commit broke the build) in a more general way. I expect Phabricator (that is, our own development process) will never reach this point and believe no project "should" have flaky tests such that a broken build is normal, but I believe this is common for large projects and that a single "build pass/fail" signal per commit is probably nearly useless for all Facebook-scale repositories. However, there's a big middle ground between where we are (relatively few tests, very high test discipline) and Facebook (many tests, low test discipline) where this is a useful signal.

Visually Understand Merges: When cherry-picking changes to stable or untangling a weird/bad merge commit, the graph view and high density of the old log can be helpful in understanding the shape of the commit history. My web workflow is:

  • Go to the repository page.
  • Possibly, click the history of stable if that's what I'm after.
  • Inspect the graph to understand what's been merged where.

Some specific information I might get out of this is:

  • Did some merge that someone committed actually make sense?
  • Did something get missed by cherry-pick?
  • Is there an obvious reason a merge isn't going through cleanly?

The CLI equivalent is git log --oneline --graph stable. However, the CLI graph is harder to read than the web graph, since it throws everything out of alignment both horizontally and vertically, and the actual graph part is crude, it can't use color as nicely, and isn't nearly as easy to decipher visually. Generally, the main advantage is that we can draw a much better graph on a <canvas /> with full-color pixels than git can on the CLI with ANSI colors and terminal characters (this is lightly colorized from the CLI, but not in a particularly useful way):

...
| * bdecff7d67 Show "objectives" UI only if prototypes are enabled
| * 6945e80fee For the diff banner, detect the current changeset better
| * 5f49f9c793 Add sound to logged out Conpherence
| * 1644b45050 Disperse task subpriorities in blocks
* | 4d2c7e4d3d (stable) Show the curent selected inline in the objective list
* | e6843b1c00 (stable) Show "reply" inlines as replies in the objective list
* | 253f7b0bb9 (stable) Show a snippet when hovering inlines in the objective list
* | aabb791c17 (stable) When a user cancels a new inline, clear it from the objective list
* | 87a4940924 (stable) Fix a diff objective issue where objectives could appear in the wrong place
* | 429a0ce4e6 (stable) Make Differential objective markers show a brighter "editing" state
* | 1bccdd69cb (stable) Stop long filenames in objective list tooltips from being cut off
* | 1495c80e62 (stable) Hide the Differential scroll objective list on trackpad systems
* | 7ee20f573d (stable) Show "objectives" UI only if prototypes are enabled
* | ab80d3ef0c (stable) For the diff banner, detect the current changeset better
* | 1d89c1ed24 (stable) Add sound to logged out Conpherence
* |   88935dec13 Manually merge "master" onto "stable"
|\ \  
| |/  
| * c6a7bcfe89 Make Pholio description behave as a remarkup field (e.g., subscribe mentioned users)
| * bbc5f79227 Make membership lock/unlock feed stories read more naturally
| * 789d57522b Make editing project images redirect to "Manage" more consistently
| * 10b3879232 Make Project slug/hashtag transactions render a little more nicely
...

Another CLI equivalent in some cases is "git show", then "git show <parent>" on each of the parents to figure out what a commit is merging. This works fine but it's a lot of commands and a lot of mixed keyboard/mouse interactions.

I don't think any arc wrapper could ever make this much better on the CLI, the basic issue is just that we can draw a way better graph on the web UI.

There's no real equivalent from the web UI anymore.

Before we added the graph to the web UI, I used the gitk tool for this. gitk looks something like this:

gitk.png (358ร—874 px, 81 KB)

I stopped using gitk once we added the graph and no longer have it installed, but could reinstall it.

I do this rarely. I think it was more common earlier in the history of the project when we saw more accidental merge commits, but we don't seem to make those very often anymore (fewer GitHub contributions, better arc land, "Land Revision" from web UI, etc).

This doesn't specifically solve any particular problem, it was just the fastest way to get a view of merges in cases where I wanted one.

In projects which do not linearize history with arc land + squash merges, I believe this feature is virtually useless because many commits are merges and visualizing the repository isn't very informative (the history has so many parent/child relationships that none of them convey much of anything).


I don't think any of these use cases are especially important or that we should necessarily accommodate them, but they are cases where the web UI was a better or faster tool than the CLI and no longer is.

Thank you, this is great, and very clear, feedback I can work from.

Overall, I prefer reading a diff over a commit message.

Me too, btw - I much prefer the Revision view of a specific change over the Commit view, because in 99% of cases, that's where the interesting discussion happens. If I'm looking at a commit, I usually want to see the Why discussion.