Skip to content

Issue/13328 13329 backup restore UI cycle 2#13919

Merged
zwarm merged 23 commits intodevelopfrom
issue/13328-13329-backup-restore-ui-cycle-2
Feb 2, 2021
Merged

Issue/13328 13329 backup restore UI cycle 2#13919
zwarm merged 23 commits intodevelopfrom
issue/13328-13329-backup-restore-ui-cycle-2

Conversation

@zwarm
Copy link
Copy Markdown
Contributor

@zwarm zwarm commented Jan 30, 2021

Parents #13328, #13329 and copy improvements #13916

This PR is part two of UI updates and includes

  • Updated copy for Restore and Backup Download
  • Updated view for status error
  • Updated icon/copy for generic error views
  • Split out building error views based on type (RemoteRequestFailure vs. GenericFailure)
  • Introduced a new common item type BACKUP_RESTORE_BULLET and associated supporting classes BulletState, and JetpackBackupRestoreBulletViewHolder. Updated builders and adapter
  • Added new icons for error views
  • Caught and fixed an issue with setting error index for restore in RestoreStep

Merge Instructions

restore status error generic error
status generic
backup download status error generic error
status generic

cc: @osullivanchris
There weren't any details for backup download status (in #13916), so I winged it - same for the icon colors in status and generic error. Please provide any updates. Thanks. :)

To test:

  • Forcing the remote request failure and generic failure states has to be done within the code manually
  • In BackupDownloadViewModel, insert the following as the first two lines fun buildDetails()
  • transitionToError(BackupDownloadErrorTypes.RemoteRequestFailure) return
  • Build and run the app
  • Turn on backup and restore feature flags
  • Navigate to Activity Log
  • Tap the more menu and tap the Download Backup option
  • Note that the Status error view is shown
  • Go back and change the two lines to:
    transitionToError(BackupDownloadErrorTypes.GenericFailure) return
  • Rebuild the app
  • Navigate to Activity Log
  • Tap the more menu and tap the Download Backup option
  • Note that the Generic error view is shown
  • In RestoreViewModel, insert the following as the first two lines fun buildDetails()
  • transitionToError(RestoreErrorTypes.RemoteRequestFailure) return
  • Build and run the app
  • Turn on backup and restore feature flags
  • Navigate to Activity Log
  • Tap the more menu and tap the Restore to this point option
  • Note that the Status error view is shown
  • Go back and change the two lines to:
    transitionToError(RestoreErrorTypes.GenericFailure) return
  • Rebuild the app
  • Navigate to Activity Log
  • Tap the more menu and tap the Download Backup option
  • Note that the Generic error view is shown

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@zwarm zwarm added this to the 16.7 milestone Jan 30, 2021
@zwarm zwarm requested review from a team and malinajirka and removed request for a team January 30, 2021 19:10
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 30, 2021 •
edited
Loading

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 30, 2021 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

This was referenced Jan 31, 2021
Closed
Closed
@malinajirka malinajirka self-assigned this Feb 1, 2021
malinajirka
malinajirka approved these changes Feb 1, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @zwarm! I've tested the flows using Charles Proxy (I haven't made any code changes). The screens look great ;).

There is just one thing which confuses me a bit. It's a minor thing, I'm just curious why it behaves this way.

  1. Open Activity Log
  2. Click on "Download backup" action
  3. Confirm the action
  4. Turn on airplane mode
  5. Notice Generic error is shown - I'd expect to see the RemoteRequestFailure, since we didn't receive any response from the server. Wdyt?

@loremattei loremattei added Tooling and removed Tooling labels Feb 1, 2021
@zwarm
Copy link
Copy Markdown
Contributor Author

zwarm commented Feb 1, 2021

There is just one thing which confuses me a bit. It's a minor thing, I'm just curious why it behaves this way.

🤔 Aah, the network error turns is handled as a Generic Error when it gets past the details view. It makes more sense to always show the Status error view when we are past the details view. I changed this in 03c513b. I'll changed restore to behave the same way in b79e73e

@zwarm zwarm mentioned this pull request Feb 1, 2021
Merged
3 tasks
@osullivanchris
Copy link
Copy Markdown

osullivanchris commented Feb 2, 2021

icon colors

Have you used ones from this link you gave me in the other PR? I actually didn't know where we would have colors defined. But the success, error, warning colours there should be what we need.

Type sizes

Similar to an earlier PR, they look a little small. I've used h6 20dp and body 16dp again.

Icons

I've just bumped them up to 32dp, I think it works better with the keyline. They looked too far from the text otherwise when I used the keyline.

Layout

All other layout details below. Note the 2dp in pink. I aligned the icon vertically with the first line of text. But I found that the text was creeping over the top of the icon visually a little bit. So I pushed the text down by 2dp to account for it. It might not play out exactly the same in code as it did in Sketch though.

specs

Base automatically changed from issue/13329-backup-download-ui-cycle-1 to develop February 2, 2021 14:07
@zwarm
Copy link
Copy Markdown
Contributor Author

zwarm commented Feb 2, 2021

icon colors

I was already using error and success, I updated the yellow to warning

Type sizes
Similar to an earlier PR, they look a little small. I've used h6 20dp and body 16dp again.

I did use H6 for the header and textAppearanceBody1 (16dp) for the text, so those are all good.

Icons
I've just bumped them up to 32dp, I think it works better with the keyline. They looked too far from the text otherwise when I used the keyline.

I updated them to 32dp.

Layout
All other layout details below. Note the 2dp in pink. I aligned the icon vertically with the first line of text. But I found that the text was creeping over the top of the icon visually a little bit. So I pushed the text down by 2dp to account for it. It might not play out exactly the same in code as it did in Sketch though.

With the icon adjustment, it looks much better without the 2 dp down. The sentence will wrap as the screen gets smaller.

Thanks.

@zwarm zwarm merged commit db0dfb9 into develop Feb 2, 2021
@zwarm zwarm deleted the issue/13328-13329-backup-restore-ui-cycle-2 branch February 2, 2021 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malinajirka malinajirka malinajirka approved these changes

Assignees

@malinajirka malinajirka

Labels

None yet

Projects

None yet

Milestone

16.7

Development

Successfully merging this pull request may close these issues.

4 participants

@zwarm @osullivanchris @malinajirka @loremattei