Skip to content
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

[Fork] PR list call to action #8965

Merged
merged 9 commits into from
Jan 24, 2020
Merged

Conversation

outofambit
Copy link
Contributor

@outofambit outofambit commented Jan 23, 2020

this is a revisiting of #8936 and #8937

Description

  • adds a version of the PR list that includes a link to view the current PR online if its in an upstream repo
  • the hope is that if someone sees the PR badge on the branch dropdown and they open the PR list looking for their PR, they'll see this link as a path forward
  • its also worth nothing this is likely a temporary design while we work towards redesigning the PR list (cc @ampinsk)

Screenshots

Screen Shot 2020-01-23 at 1 12 24 PM

Release notes

Notes: [Added] Link in Pull Request list to view the current pull request on GitHub

Testing notes

  1. Select a fork in desktop
  2. Open a pull request from your current branch that targets the upstream repository (aka make a PR from a branch in outofambit/desktop to be merged into desktop/desktop)
  3. See pr badge in branch dropdown button
  4. Open branch list to PR tab and see new message

@outofambit outofambit added this to In Progress PRs in Desktop 2.3 release via automation Jan 23, 2020
@outofambit outofambit moved this from In Progress PRs to Awaiting Review in Desktop 2.3 release Jan 23, 2020
Desktop 2.3 release automation moved this from Awaiting Review to Review In Progress Jan 23, 2020
Copy link
Contributor

@ampinsk ampinsk left a comment

Choose a reason for hiding this comment

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

Thanks for working through this together @outofambit! ✨

app/src/ui/branches/no-pull-requests.tsx Show resolved Hide resolved
@tierninho
Copy link
Contributor

Possible to outline the repro steps for this? I attempted to recreate this but ran into some blockers as the following are assumed to be true:

  • the user has to have no PRs open in the forked repo to see the new messaging
  • all upstream PR badges remain hidden

However, in your screenshot, it shows a PR badge. Where did it come from?

@outofambit
Copy link
Contributor Author

@tierninho i added steps to the PR description! hopefully that helps, lmk if you have more questions! (the pr badge in the branch dropdown is displayed if a matching pr is found in the fork or its upstream)

@tierninho
Copy link
Contributor

Thanks for the steps. I can now see the link however it disappears and goes back to the original default state after clicking a variety of buttons, like the PR refresh button, fetch button, switch back/forth with branches tab.

From:
Screen Shot 2020-01-23 at 12 27 16 PM

to:
Screen Shot 2020-01-23 at 12 36 13 PM

The messaging returns if I go to another repo and return.

@tierninho tierninho self-requested a review January 24, 2020 00:35
Copy link
Contributor

@tierninho tierninho left a comment

Choose a reason for hiding this comment

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

Things seem to be working great now ✅ after the latest round of changes.

Copy link
Contributor

@kuychaco kuychaco left a comment

Choose a reason for hiding this comment

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

Thanks @outofambit 🚀. Thanks for the thorough testing @tierninho

@outofambit outofambit dismissed ampinsk’s stale review January 24, 2020 16:31

changes implemented

@outofambit outofambit merged commit 2be1afc into development Jan 24, 2020
Desktop 2.3 release automation moved this from Review In Progress to PRs For Next Beta Jan 24, 2020
@outofambit outofambit deleted the feature/pr-list-cta-stopgap branch January 24, 2020 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Desktop 2.3 release
PRs For Next Beta
Development

Successfully merging this pull request may close these issues.

None yet

4 participants