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

Onboarding improvements: No local changes blank slate #6445

Merged
merged 49 commits into from
Jan 11, 2019

Conversation

niik
Copy link
Member

@niik niik commented Dec 18, 2018

Overview

This is part of the onboarding improvements (see #6365 and #5686)

Description

The mockups

image

Possible other actions

image

Current state

image

Release notes

Provides more obvious paths for what users are able to do when there are no uncommitted changes

TODO

  • Wire up the repository remote actions
  • Documentation
  • Testing
  • Animations when actions appear/disappear? Punt in this PR, implemented separately.

cc @billygriffin

@billygriffin billygriffin added this to the 1.6.0 milestone Dec 19, 2018
@outofambit outofambit changed the base branch from master to development December 27, 2018 21:50
@niik niik added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 9, 2019
@niik
Copy link
Member Author

niik commented Jan 9, 2019

I know this is still inflight, but just wanted to point out that when a modal/dialogue is present, the background text disappears. Not sure if intentional or not...

A temporary artefact pending support to disable the action buttons when actions weren't available. This has been addressed.

@outofambit outofambit self-requested a review January 9, 2019 18:24
@tierninho
Copy link
Contributor

All tests passing as of 760187f. 🏁

Should there be a scroll bar when zoomed in? I am not allowed to scroll at all.

app/src/lib/menu-item.ts Show resolved Hide resolved
app/src/ui/changes/no-changes.tsx Show resolved Hide resolved
<h1>No local changes</h1>
<p>
You have no uncommitted changes in your repository! Here are
some friendly suggestions for what to do next.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
some friendly suggestions for what to do next.
some suggestions for you:

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you provide some context as to why you want to make this change? Suggested changes are lovely for typos or easy to spot mistakes but don't convey enough context on their own.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggested these changes because I thought my changed version read better, but I don’t feel strongly either way.

@niik
Copy link
Member Author

niik commented Jan 10, 2019

Should there be a scroll bar when zoomed in? I am not allowed to scroll at all.

@tierninho Good shout. I forgot about zooming. I took a stab at addressing this and added some responsive styling at high zoom levels to make it at least somewhat usable at the min window size.

Copy link
Contributor

@outofambit outofambit left a comment

Choose a reason for hiding this comment

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

This all looks fine to me! I chatted with @niik about some future refactoring we could do to remove the dependency on Menu for these actions but that is a larger project we can address in the future.

@outofambit outofambit self-assigned this Jan 10, 2019
@shiftkey shiftkey merged commit db53079 into development Jan 11, 2019
@shiftkey shiftkey deleted the no-changes-blankslate branch January 11, 2019 11:15
@tierninho
Copy link
Contributor

Noticed that clicking from outside of the Desktop app into one of the new onboarding buttons is not functional. Not sure if we have rules around this, but wanted to point it out.

@shiftkey
Copy link
Member

@tierninho which OS?

@tierninho
Copy link
Contributor

Mac 10.14, v1.6.0-beta2

@shiftkey
Copy link
Member

shiftkey commented Jan 14, 2019

@tierninho the reason why I ask is that ages ago we added #3843 which was a specific Electron flag around focus and handling that first click, and I'd like to figure out why that doesn't seem to work with this change.

I did a quick test of beta2 on macOS (10.13 actually) and I was seeing the button immediately handle the click while also accepting focus. If you can elaborate on what you're seeing and expecting to see, we can dig into it further.

@tierninho
Copy link
Contributor

@shiftkey seeing it work now too. Must have been a one-off fluke with my setup. Sorry for the false alarm.

@niik niik mentioned this pull request Jan 16, 2019
@svivian
Copy link

svivian commented Jan 18, 2019

Is it possible to not have this constantly popping up when there are changes? Every time I click a file in the "Changes" tab this screen flashes up for half a second before showing the expected diff. It wasn't so bad before when it was a light grey screen with small text, but now it's super jarring.

@outofambit
Copy link
Contributor

Hi @svivian thanks for sharing this feedback. We're tracking that in #6658 if you'd like to follow along there for updates. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants