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

Jump to bottom

Update on-boarding UI #1099

Merged
merged 16 commits into from Sep 15, 2020
Merged

Update on-boarding UI #1099

merged 16 commits into from Sep 15, 2020

Conversation

Skrilltrax
Copy link
Member

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

📜 Description

Onboarding UI changes.

💡 Motivation and Context

Our previous onboarding UI was a bunch of dialogs inside a single activity. This new UI makes it easy to extend the onboarding activity to add new features.

💚 How did you test it?

Tested all the onboarding options.

📝 Checklist

  • I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

🔮 Next steps

📸 Screenshots / GIFs

@Skrilltrax
Add onboarding flow from v2
f75b5e8 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@Skrilltrax
Minor fixes
9cb72b5 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@Skrilltrax
Add changelog entry
ddcf6ee 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@Skrilltrax
Copy link
Member Author

I'm following Buffer's package structure here. We should fix all our packages like this once we are ready to push V2.

msfjarvis
msfjarvis requested changes Sep 14, 2020
View reviewed changes
app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
app/src/main/java/com/zeapo/pwdstore/ui/onboarding/fragments/CloneFragment.kt Outdated Show resolved Hide resolved
app/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved
app/src/main/java/com/zeapo/pwdstore/ui/onboarding/fragments/RepoLocationFragment.kt Outdated Show resolved Hide resolved
app/src/main/java/com/zeapo/pwdstore/ui/onboarding/fragments/RepoLocationFragment.kt Outdated Show resolved Hide resolved
app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt Outdated Show resolved Hide resolved
app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt Outdated Show resolved Hide resolved
app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt Outdated Show resolved Hide resolved
app/src/main/java/com/zeapo/pwdstore/utils/Extensions.kt Outdated Show resolved Hide resolved
app/src/main/res/layout/fragment_repo_location.xml Outdated Show resolved Hide resolved
Skrilltrax and others added 6 commits September 14, 2020 22:34
@Skrilltrax
Remove view type prefix from view ids
929584c 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@Skrilltrax
Review fixes
7d7e7c3 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@Skrilltrax
Treewide: Reformat code
348ef00 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@Skrilltrax
Moar review fixes
93b1a9c 
Signed-off-by: Aditya Wasan <adityawasan55@gmail.com>
@msfjarvis
Merge branch 'develop' into onboarding-ui
baec537 
* develop:
  Add tentative workaround for dialog crashes and refactor Git-related code (#1100)

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Revert "Treewide: Reformat code"
f3360fd 
This reverts commit 348ef00.

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Member

Removed reformat commit since it added too many changes to an already big PR, it can go in separately later.

@msfjarvis msfjarvis self-requested a review September 15, 2020 12:17
@msfjarvis
onboarding: cleanup OnboardingActivity init
60ca6e9 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Remove unused layout
da2b9ce 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Remove unnecessary ConstraintLayout
9bcab3d 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Shorten animation duration
9a6937e 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
onboarding: use viewBinding extension in fragments
4999677 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis added this to the 1.12.0 milestone Sep 15, 2020
msfjarvis
msfjarvis approved these changes Sep 15, 2020
View reviewed changes
Copy link
Member

@msfjarvis msfjarvis left a comment

Choose a reason for hiding this comment

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

Everything appears to work in my testing

@Skrilltrax Skrilltrax merged commit 4ba3b75 into develop Sep 15, 2020
@Skrilltrax Skrilltrax deleted the onboarding-ui branch September 15, 2020 16:23
msfjarvis added a commit to Valodim/Android-Password-Store that referenced this pull request Sep 15, 2020
@msfjarvis
Merge branch 'develop' into autofill-subproject
d023ad6 
* develop:
  Update on-boarding UI (android-password-store#1099)
@fmeum
Copy link
Member

fmeum commented Sep 24, 2020

While updating the Wiki entries, I noticed that it doesn't seem to be possible anymore to replace an existing SSH key from the on-boarding UI. Should we reintroduce the access to settings or just reset it if the repository is deleted?

@msfjarvis
Copy link
Member

While updating the Wiki entries, I noticed that it doesn't seem to be possible anymore to replace an existing SSH key from the on-boarding UI. Should we reintroduce the access to settings or just reset it if the repository is deleted?

I'm not a fan of either option but adding a settings option feels like the less-likely-to-be-annoying choice w.r.t. UX.

@fmeum
Copy link
Member

fmeum commented Sep 24, 2020

While updating the Wiki entries, I noticed that it doesn't seem to be possible anymore to replace an existing SSH key from the on-boarding UI. Should we reintroduce the access to settings or just reset it if the repository is deleted?

I'm not a fan of either option but adding a settings option feels like the less-likely-to-be-annoying choice w.r.t. UX.

Agreed. @Skrilltrax Is this something you could add before the release? I will update the remaining Wiki entries.

@msfjarvis
Copy link
Member

While updating the Wiki entries, I noticed that it doesn't seem to be possible anymore to replace an existing SSH key from the on-boarding UI. Should we reintroduce the access to settings or just reset it if the repository is deleted?

I'm not a fan of either option but adding a settings option feels like the less-likely-to-be-annoying choice w.r.t. UX.

Agreed. @Skrilltrax Is this something you could add before the release? I will update the remaining Wiki entries.

He's got a fair bit of stuff going on at work, I'll do it.

@fmeum
Copy link
Member

fmeum commented Sep 24, 2020

While updating the Wiki entries, I noticed that it doesn't seem to be possible anymore to replace an existing SSH key from the on-boarding UI. Should we reintroduce the access to settings or just reset it if the repository is deleted?

I'm not a fan of either option but adding a settings option feels like the less-likely-to-be-annoying choice w.r.t. UX.

Agreed. @Skrilltrax Is this something you could add before the release? I will update the remaining Wiki entries.

He's got a fair bit of stuff going on at work, I'll do it.

Thanks! The Wiki entries should now reflect the flow for 1.12.0. I have also made them recommend in-app generation of SSH keys.

@msfjarvis
Copy link
Member

While updating the Wiki entries, I noticed that it doesn't seem to be possible anymore to replace an existing SSH key from the on-boarding UI. Should we reintroduce the access to settings or just reset it if the repository is deleted?

I'm not a fan of either option but adding a settings option feels like the less-likely-to-be-annoying choice w.r.t. UX.

Agreed. @Skrilltrax Is this something you could add before the release? I will update the remaining Wiki entries.

He's got a fair bit of stuff going on at work, I'll do it.

Thanks! The Wiki entries should now reflect the flow for 1.12.0. I have also made them recommend in-app generation of SSH keys.

The Wiki changes look good, I've created #1119 to add the settings button.

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

@msfjarvis msfjarvis msfjarvis approved these changes

@fmeum fmeum Awaiting requested review from fmeum

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Skrilltrax @msfjarvis @fmeum