New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

discoverability improvements #1704

Merged
merged 6 commits into from Sep 27, 2018

Conversation

Projects
None yet
4 participants
@annthurium
Contributor

annthurium commented Sep 25, 2018

Description of the Change

There are a lot more Atom users than there are users of the Git/Github package. This points to a discoverability problem. There are menus and commands to open the Git/GitHub panes, but in order to do that you have to know the panes exist.

This pull request changes the icon and text in the status bar to say "Git (n)" where n is the number of changed files, and the icon is a diff icon. This pull request also adds a GitHub icon and text to the status bar, which toggles the GitHub tab when clicked.

screen shot 2018-09-25 at 3 08 04 pm

Alternate Designs

I thought about not showing the GitHub status bar icon if there are no GitHub.com remotes present. The GitHub tab isn't very useful in that case. It's not really doing any harm to show the GitHub tab -- it's just not a great use of real estate. Thus, I decided the complexity of hiding the icon wasn't worth the hassle.

Benefits

Hopefully, more users will learn that the git and github panes exist, use them, and find the meaning that's been missing from their lives.

We plan on doing usability testing to validate the assumption that the UI improves discoverability.

Possible Drawbacks

If we keep adding stuff to the status bar, eventually it's going to get too crowded. For right now, it's fine.

Applicable Issues

#1573

Metrics

When a user clicks on the GitHub status bar icon, we record an event.

Tests

Unit tests to validate that...

  • ChangedFilesCountView:
  • renders the new icon
  • renders the text "Git"
  • GithubTileView:
    • renders the new icon
    • renders the text "GitHub"
    • records an event when clicked
    • calls the passed in prop when clicked
  • StatusBarTileController:
    • toggles the GitHub tab when clicked

Manual testing to validate that...

  • the git tab shows / hides when the status bar icon is clicked
  • the github tab shows / hides when the status bar icon is clicked
  • clicking the github tab doesn't cause any errors if no github.com remotes are present.
@coveralls

This comment has been minimized.

coveralls commented Sep 25, 2018

Coverage Status

Coverage decreased (-0.05%) to 81.946% when pulling 6a49d36 on tt-18-sept-discoverability into 1b4c899 on master.

@kuychaco

Excited for this! Hopefully it helps a lot with discoverability. Thanks for spearheading these efforts @annthurium!

@@ -5,6 +5,7 @@ import BranchView from '../views/branch-view';
import BranchMenuView from '../views/branch-menu-view';
import PushPullView from '../views/push-pull-view';
import ChangedFilesCountView from '../views/changed-files-count-view';
import GithubStatusBarTile from '../views/github-status-bar-tile';

This comment has been minimized.

@kuychaco

kuychaco Sep 26, 2018

Member

Here's a suggestion for the class name based on package convention (ending with the word "view") and context (being in the status-bar-tile-controller.js file) -- GitHubTileView. And the file could be github-tile-view.js.

Similarly, since you changed the tile for opening the Git panel it might be more appropriately named something like GitTileView and the file git-tile-view.js.

Corresponding test files would need to be changed accordingly.

This comment has been minimized.

@annthurium

annthurium Sep 26, 2018

Contributor

good call! I'll change the name.

<Octicon icon="diff" />
{label}
<Octicon icon="git-commit" />
{`Git (${this.props.changedFilesCount})`}

This comment has been minimized.

@kuychaco

kuychaco Sep 26, 2018

Member

This pull request changes the icon and text in the status bar to say "Git (n)" where n is the number of changed files

Interestingly, when I first saw this my initial thought was that n was the number of git commits... I notice currently we have the number as well as the word "Files" which makes it very clear that the number refers to the changed file count. I wonder if there's a way we could make it more clear with your new version... adding the word "Files" might take up too much space. I think it would even be fine to just drop the file count...

@simurai do you have 💭s?

This comment has been minimized.

@simurai

simurai Sep 26, 2018

Member

I think it would even be fine to just drop the file count...

That would be the least confusing, but we'd miss out on informing the user that there are uncommitted changes, which might be important information to keep.

adding the word "Files" might take up too much space.

screen shot 2018-09-26 at 2 21 53 pm

Hmm.. yeah.

Or how do you like some sort of "notification badge":

image

Atom IDE uses something similar. It could also be just grey to be a bit more subtle.

This comment has been minimized.

@annthurium

annthurium Sep 26, 2018

Contributor

Good points, you two! Some thoughts...

  • Anecdotal data point: I showed my partner the new interface and asked what he thought Git (2) referred to. He correctly guessed "the number of uncommitted changes I have made." I asked if having that information at a glance would be useful, and he said yes. When I showed him the old interface, he was not sure what the 2 files referred to.

  • I like the bright color on the badge and think it might encourage users to click, but if the goal is to clarify what "2" refers to it doesn't help.

  • I'd like to ask users about this in the context of an actual research session and let that inform the road we choose to take. If they don't know what the changed file count refers to but would find that information useful, we can add the word "files" back in. It's slightly crowded but not horrible. If they wouldn't find that information useful, we can drop it entirely. Sound good?

This comment has been minimized.

@kuychaco

kuychaco Sep 27, 2018

Member

Sounds good to me! Would we do this UXR prior to merging? Or just prior to releasing?

Edit: I see that your intention is to merge soon, so the answer is not prior to merging. I'm totally cool with that. Just curious what our plan would be for doing this research before shipping.

This comment has been minimized.

@annthurium

annthurium Sep 27, 2018

Contributor

good question. my intention is to do research before next release, with enough time to make additional changes if necessary. Since we have a month before next release I think that's do-able. I finished the uxr script just now, so next I'll focus on rounding up some users.

}
handleClick() {
addEvent('click', {package: 'github', component: 'GithubStatusBarTile'});

This comment has been minimized.

@kuychaco

kuychaco Sep 26, 2018

Member

Not sure what convention we're going with for event names (or if we have even discussed one yet)... I had the thought that a more descriptive event name could be handy, something like toggle-github-tab. That way whoever is combing through the data we're collecting can more easily infer what the events are for in terms of extracting information about user behavior.

This comment has been minimized.

@kuychaco

kuychaco Sep 26, 2018

Member

That said, I can also see the value in simply stating that we're capturing a click event on a particular view element...

Oh okay, so we want to decouple the UI action (click on an element) from the effect of the action (toggling the tab). We've instrumented the reveal and hide methods of the TabTracker which tells us the number of times a given tab is opened/closed. We also want to know what method was used to reveal/hide the tab, which is what we're doing here. Does that sounds about right?

This comment has been minimized.

@annthurium

annthurium Sep 26, 2018

Contributor

we already record an event for toggling the git and github tabs, in the RootController. 😸

This additional event allows us to know that the button is clicked. If the tabs are toggled a lot, but these buttons are never clicked, we have an additional data point to know that these buttons aren't helping users discover the tabs.

<button
className="github-StatusBarTile inline-block"
onClick={this.handleClick}>
<Octicon icon="mark-github" />

This comment has been minimized.

@kuychaco
@@ -88,6 +94,7 @@ export default class StatusBarTileController extends React.Component {
return (
<Fragment>
{this.renderTiles(repoProps)}
<GithubStatusBarTile didClick={this.props.toggleGithubTab} />

This comment has been minimized.

@simurai

simurai Sep 26, 2018

Member

How about moving the GitHub tile to the far right?

screen shot 2018-09-26 at 1 55 36 pm

Then it's more in order of this typical 🤔 workflow:

  1. Choose a branch
  2. Make sure it's up-to-date
  3. Make changes
  4. Create PR

This comment has been minimized.

@annthurium

annthurium Sep 26, 2018

Contributor

@simurai if we move the GitHub tile to the far right, we run the risk of disrupting flows for users who have grown accustomed to "click on the thing on the far right" to open the Git pane.

Then again, this would force those users to discover the GitHub pane. 😆

This comment has been minimized.

@simurai

simurai Oct 1, 2018

Member

@annthurium That's a good point, although the "far right" sometimes gets used by the "updates" and "blue squirrel".

Another option would be to move "GitHub" to the left, before the branch name:

image

Just so that "GitHub" doesn't get sandwiched between the Git "actions" and the Git tab.

@annthurium

This comment has been minimized.

Contributor

annthurium commented Sep 27, 2018

@simurai @kuychaco I'm going to merge this, as I believe I have addressed all feedback. I am super open to addressing additional concerns in the future, if you have any.

@annthurium annthurium merged commit 157c5a8 into master Sep 27, 2018

6 checks passed

ci/circleci: beta Your tests passed on CircleCI!
Details
ci/circleci: dev Your tests passed on CircleCI!
Details
ci/circleci: stable Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.05%) to 81.946%
Details

@annthurium annthurium deleted the tt-18-sept-discoverability branch Sep 27, 2018

@annthurium annthurium referenced this pull request Oct 20, 2018

Closed

v0.21.0-0 QA Review #1752

7 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment