Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented Mar 21, 2018

What this PR does

This PR gives users a hint that the Navigate to Editor functionality exists.

It adds the following messages:

  1. When a PR file diff gets focus
    image

  2. When a PR file diff gets focus from a PR that isn't checked out
    image

  3. When user attempts to navigate to a file that isn't checked out
    image

  4. When the diff file focus is lost the status is cleared (showing the default Ready message)
    image

  • This also works when using View File to view a (read-only) PR file

Reviewers

  • What do you think of the wording? Is my alternative suggestion above an improvement?
  • Do you think the status bar message is noticeable enough without being overly distracting?

Todo

  • Finalize the wording. Would something like this be better?
    • Press Enter to navigate to Editor (PR branch must be checked out)
  • ~~It would be nice if the dialog gave Checkout and Cancel options~~~
    • Maybe do this in a separate PR

Part of #1491
Fixes #1546

When a PR is checked out and the diff view gets focus, show a navigate to editor hint on status bar.
Let the user know they need to check out PR branch before navigating to editor.
@jcansdale jcansdale force-pushed the fixes/1546-make-navigate-to-editor-discoverable branch from 746abd7 to 2b46a6c Compare April 10, 2018 12:38
@jcansdale jcansdale changed the base branch from master to feature/pr-reviews-master April 10, 2018 13:46
Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@meaghanlewis
Copy link
Contributor

Have been dogfooding this and I think the message is helpful. It's not at all distracting, because the bar is always there- I'm just used of it only saying Ready.

@grokys grokys mentioned this pull request Apr 16, 2018
17 tasks
@jcansdale jcansdale merged commit c2b105e into feature/pr-reviews-master Apr 16, 2018
@jcansdale jcansdale deleted the fixes/1546-make-navigate-to-editor-discoverable branch April 16, 2018 09:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants