Skip to content

Offer to add extra fields during pr create flow#768

Closed
mislav wants to merge 2 commits intomasterfrom
pr-create-prototype-B
Closed

Offer to add extra fields during pr create flow#768
mislav wants to merge 2 commits intomasterfrom
pr-create-prototype-B

Conversation

@mislav
Copy link
Copy Markdown
Contributor

@mislav mislav commented Apr 9, 2020

This prototypes a flow during gh pr create that asks the user whether they would like to also select reviewers, assignees, labels, projects, and milestone. #340

This branch encapsulates "Option B" from our planning docs. "Option E" is found in the pr-create-prototype-E branch.

How to test:

  • Check out this branch
  • Run make
  • Start bin/gh pr create - in this prototype, no PR will ever be submitted, so you are safe to choose "Submit"!

Screen Shot 2020-04-09 at 8 20 13 PM

Note: fake values (i.e. not read from the repository) are used as a list of available options.

To do:

  • seed the prototype with thousands of reviewer/assignee entries to check how it fares with large numbers

@vilmibm
Copy link
Copy Markdown
Contributor

vilmibm commented Apr 9, 2020

Thank you for putting this together! Prototypes are incredibly helpful and I'm really appreciative that you made this one so easy to interact with.

Unordered thoughts:

  • I like E a lot more than I expected to. I still think B feels more "natural" and have a preference for it but I'm a lot more open to E now. I find it's easier to skip past what I don't want (as in B) than think ahead to what I might want (as in E)
  • I'm still really worried about how we'll handle orgs the size of github, for example. Can we make use of the Suggested Reviewers thing through graphql?
  • I like the inclusion of Project selection. I'd love to be able to choose the right column also but I guess automation can be trusted for that.

@tierninho
Copy link
Copy Markdown
Contributor

Tested this and works great. Getting used to using both the space and return keys for entering took a second to get used to.

I'm still really worried about how we'll handle orgs the size of github, for example. Can we make use of the Suggested Reviewers thing through graphql?

Good point, but I always find Suggested Reviewers to be missing some folks.

@mislav
Copy link
Copy Markdown
Contributor Author

mislav commented Apr 10, 2020

I've pushed another change to prototype B (this branch) that lists 10,000 (fake) users and teams as potential assignees/reviewers to see how the multi-select picker fares. Only 7 items are displayed at maximum, but we can reconfigure that number based on our needs:
Screen Shot 2020-04-10 at 5 40 13 PM
filtering can be achieved by typing e.g. "lucy":
Screen Shot 2020-04-10 at 5 40 03 PM

After selecting an item with Space, the current filter seems cleared, but there is also an option to reconfigure that and keep the filter after selecting an item (the user would then have to clear the filter themselves to return to the pristine state).

Some technical notes:

  • In addition to login handles/team names, I've chosen to display full user names and team descriptions to make it easier to find a specific person or team, but the length of some lines (especially due to team descriptions) triggers the display bug described in Prompt displays several times #418. We could work around this by measuring terminal width before rendering out the picker and truncating long lines.

  • After selecting some people and/or teams, the prompt is updated with the current selection before moving on to the next prompt, but right now that turns out to be overly verbose, so we would likely have to tweak or fork the current MultiSelect implementation to iron out how the selection is displayed.

  • Current filtering algorithm isn't "fuzzy" like the one in dotcom web UI sidebar, but we could improve it to be fuzzy if we wanted thoeng to match, for example “Thomas the Tank Engine”. I personally really prefer fuzzy filters for data such as names.

  • There is a Suggested Reviewers API, but for some reason it's only available for already existing PRs. It doesn't seem possible right now to query sugggested reviewers list for a certain changeset before a PR is created.

  • Even though the display of thousands of items seems well-handled, we still have to keep in mind that obtaining the data from a large org might take a while. For example, a query that lists all the members of the github org takes 8s for me to complete, and that doesn't even include teams. In order to avoid long wait times, we could consider capping the pagination limit for large organizations and printing a warning message that not all options will not be displayed and that they should select assignees & reviewers through the web UI.


Personal preference: I definitely lean to Option E and that is primarily because avoiding displaying any picker for assignees/reviewers/labels/projects/milestone unless one is specifically asked for also allows us to avoid querying relevant data required to seed the picker. With Option B, we would have to query a lot of data that the user might even just skip through in a lot of cases.

An approach I might even prefer more would be this: instead of asking “Would you like to add more information”, we would show the final confirmation prompt with “Submit”, “Preview”, “cancel”, but we would also show a menu of items accessible via keyboard shortcuts: A to enter assignees picker, L to enter label picker, and so on. These keyboard shortcuts would match those in the web UI. That way I could, for example, train my muscle memory to always hit "A" when I want to additionally assign people/teams to an issue or PR instead of having to visually hunt down that option through the mutli-select of prototype E.

@vilmibm
Copy link
Copy Markdown
Contributor

vilmibm commented Apr 10, 2020

Current filtering algorithm isn't "fuzzy" like the one in dotcom web UI sidebar, but we could improve it to be fuzzy if we wanted thoeng to match, for example “Thomas the Tank Engine”. I personally really prefer fuzzy filters for data such as names.

non fuzzy seems ok to start.

There is a Suggested Reviewers API, but for some reason it's only available for already existing PRs. It doesn't seem possible right now to query sugggested reviewers list for a certain changeset before a PR is created.

I guess I assumed Suggested Reviewers was static for PRs (and depended on the viewer+repo) but it makes sense that it's per-PR. Oh well.

train my muscle memory to always hit "A" when I want to additionally assign people/teams to an issue or PR instead of having to visually hunt down that option through the mutli-select of prototype E.

Big +1 to this kind of approach! We dipped our toes in this with the body edit prompt (e to edit) and I think it feels really natural. I'd love to see it prototyped; what do you think, @ampinsk ?

@ampinsk
Copy link
Copy Markdown

ampinsk commented Apr 10, 2020

Thank you so much for all of this @mislav! ✨ I'm going to play some more today with the new fake data.

From playing with it yesterday, I agree with a lot of the points already made. Basically, I think Option B is nice to think a little less, but it feels tedious to go through everything. Option E felt good but agree it was a little hard to anticipate what I needed ahead of time.

An approach I might even prefer more would be this: instead of asking “Would you like to add more information”, we would show the final confirmation prompt with “Submit”, “Preview”, “cancel”, but we would also show a menu of items accessible via keyboard shortcuts: A to enter assignees picker, L to enter label picker, and so on. These keyboard shortcuts would match those in the web UI. That way I could, for example, train my muscle memory to always hit "A" when I want to additionally assign people/teams to an issue or PR instead of having to visually hunt down that option through the mutli-select of prototype E.

I'm having a little trouble picturing this, so I would love to see a sketch or prototype of it! I didn't know about the keyboard shortcuts on dotcom! 🤯

I also tried out this sketch as another way to integrate Option E into the What's next? prompt, curious to hear thoughts. I think this is similar to @mislav's idea?

Screen Shot 2020-04-10 at 10 41 16 AM

@mislav
Copy link
Copy Markdown
Contributor Author

mislav commented Apr 13, 2020

I also tried out this sketch as another way to integrate Option E into the What's next? prompt, curious to hear thoughts.

I like this! I think it's a good compromise to allow adding more metadata just before submitting.

I will also try to protype my keyboard shortcut-based approach tomorrow so we can all get a sense of how that feels.

I just realized something: we should not allow “Preview in browser” after any assignees/reviewers/labels/projects/milestone have been selected. This is because it's impossible to send these via URL parameters for web-based pull request creation, so anything that the user selected so far would be lost. We learned this when people noticed that --draft isn't respected in web mode #558

@mislav
Copy link
Copy Markdown
Contributor Author

mislav commented Apr 14, 2020

I've pushed another prototype: pr-create-prototype-E2
Screen Shot 2020-04-14 at 8 28 55 PM

Instead of navigating with arrow keys or the ability to make multiple selections for metadata, this one just runs a loop where is listens for a single character with instructions whether to Submit, Cancel, or add metadata with shortcuts that match the github.com UI. (The final product doesn't necessarily have to match the web UI; I just thought it would be a nice touch.)

This feels— awkward, especially because it's so unpolished and feels like a very different interaction paradigm than the Survey library we were using so far. However, we could apply some polish to this to reduce the dis-orientation that the user might experience after several cycles of adding metadata, plus adding visual confirmation that metadata was added. I'd like to hear your thoughts!

@ampinsk
Copy link
Copy Markdown

ampinsk commented Apr 15, 2020

Ok after noodling around with all of our options and chatting through it with @billygriffin, here's what I'm proposing:

Screen Shot 2020-04-15 at 3 59 30 PM

To outline the steps here:

  1. Prompt title and body, as normal today
  2. In the What's next? step:
    • Move Submit to be first option
    • Reword Preview in browser to Continue in browser
    • Add Add metadata option
  3. Add metadata option will take you through each piece of metadata in the sidebar
    • We should make the instructions on the multiselect options clear that Enter means continue
  4. In the final What's next? step after entering metadata, we only give you the option to Submit or Cancel

Does this make sense? How does this sound?


One general concern I have is the interaction for searching in the multiselect options when there are long lists:

multi-search

I tried to show this gif that when I search and select a name, it's really unclear that I selected that name. When you hit space, it just resets back to the top of the list, which is really jarring. I know this is probably from survey but I'm curious if there are ways we could make this interaction a little clearer. I also think this could be follow up work! But wanted to raise it 😄

@mislav
Copy link
Copy Markdown
Contributor Author

mislav commented Apr 17, 2020

  • In the What's next? step:

    • Move Submit to be first option
    • Reword Preview in browser to Continue in browser
    • Add Add metadata option
  • Add metadata option will take you through each piece of metadata in the sidebar

    • We should make the instructions on the multiselect options clear that Enter means continue

@ampinsk That sounds good! I will start with implementing this approach.

My biggest concern is still with my point above about load times/performance:

Even though the display of thousands of items seems well-handled, we still have to keep in mind that obtaining the data from a large org might take a while.

If a person just wants to add labels, they still have to skip through assignees/reviewers, and to display those we have to fetch all users in an organization, which for large organizations can take a while (~10 seconds for github org). We could resort to some workarounds (in order of increasing complexity):

  • Cap the number of users we preload to a fixed number, e.g. 100
  • Cache per-repository results on disk
  • Show first "page" of results while loading the rest in the background

Additionally, should we include org projects in the projects picker? If so, then we have similar loading considerations as with org members/teams.

@ampinsk
Copy link
Copy Markdown

ampinsk commented Apr 17, 2020

@mislav thank you for bringing that up! apologies I honestly just totally forgot about this point 🤦‍♀️ Yes, considering the performance concerns and that I was on the fence experience wise anyway, let's go with asking ahead of time.

Here's an updated sketch to reflect that:

Screen Shot 2020-04-17 at 12 12 48 PM

We should probably support Org projects too. I'm also curious how we can optimize these or make these lists more intuitive, manageable and performant 🤔

@vilmibm
Copy link
Copy Markdown
Contributor

vilmibm commented Apr 29, 2020

As this was for prototyping and the work has since been opened in other PRs, I'm going to close this.

@vilmibm vilmibm closed this Apr 29, 2020
@mislav mislav deleted the pr-create-prototype-B branch May 11, 2020 18:21
@zigang93
Copy link
Copy Markdown

zigang93 commented Sep 2, 2020

does it able to add linked issue number with meta?

@mislav
Copy link
Copy Markdown
Contributor Author

mislav commented Sep 2, 2020

@zigang93 No, but you can add a linked issue number by including something like #1234 in the text of the pull request body.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants