Jetpack Mobile: Scan UiStates part(ii) by ashiagr · Pull Request #13605 · wordpress-mobile/WordPress-Android

ashiagr · 2020-12-15T12:28:07Z

PS: Size of the PR has increased because of new xml layouts, extracting of styles to a new class. I can split tests (last commit) into a separate PR if it helps in reviewing this PR.

Description

This PR is a step forward in refining UiStates for the Scan screen. It mainly includes changes to below classes:

ScanListItemState updated to include

ScanState - ButtonAction (FixAll, Scan) - (only display logic) do not perform any action yet.
ThreatItemState (corresponding ThreatViewHolder, layout etc added)

ScanStateListItemBuilder

Mapping for ScanState extracted in the new ScanStateListItemBuilder class, dynamic texts added

ScanViewModel

Content UiState updated to include ScanState buttons and threats

There will be more states (e.g. after fix action) later, this PR covers only below states, dynamic texts, buttons (displayed when screen is shown) and corresponding tests.

ScanStateModel.State	SCANNING	IDLE	IDLE
ScanListItemState	Scanning	ThreatsNotFound	ThreatsFound (none, some or all fixable)
icon	ic_scan_scanning	ic_scan_idle_threats_not_found	ic_scan_idle_threats_found
title	Preparing to scan	Don't worry about a thing	Your site may be at risk
description	Welcome to Jetpack Scan, we are taking a first look at your site now and the result will be with you soon. \n \n We will send you a notification once the scan is complete. In the meantime feel free to continue to use your site as normal. You can check the progress at anytime.	The last Jetpack scan ran {no. of hours/mins ago or in a few seconds} and everything looked great. Run a manual scan now or wait for Jetpack to scan your site later today.	The scan found {no. of threats} potential threats with {site name or this site}. Please review them below and take action. We're {here to help} if you need us.
fix all button	-	-	Fix all (visibility based on fixable status)
scan button	-	Scan now	Scan again

To Test

Prerequisite: Make sure both Scan feature flag and MySiteImprovements flag are set to on in the App settings.

Select a site with scan capability on the My Site tab
Notice that scan menu is displayed
Click scan menu item
Notice that icons, (dynamic) texts, (dynamic) buttons display as per the table above corresponding to the mentioned states.
Notice that threats have started displaying (title, description based on threat type yet to be updated)

Notes

Design, icons not final. Ignore all styles.
ScanStatusService uses FluxC.ThreatStore to access threats (only basic details) temporarily just to start displaying the threats, threats count. It needs to be updated later on and so haven't included tests for ScanStatusService.
"here to help" in the ThreatsFound state is not highlighted as a link.

Merge Instructions

Make sure Jetpack Mobile: Scan Introduce UiState #13578 is merged
Ping me for merge. We need to remove fluxc commit hash for getting threats temporarily.

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.

…nd introduce scan styles

… if threats available

peril-wordpress-mobile · 2020-12-15T12:28:13Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

peril-wordpress-mobile · 2020-12-15T12:28:14Z

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

peril-wordpress-mobile · 2020-12-15T12:37:02Z

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

…scan-ui-states-partii # Conflicts: # WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/ScanFragment.kt # WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/adapters/ScanAdapter.kt # WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/adapters/viewholders/ScanStateViewHolder.kt # WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/ScanStatusServiceTest.kt # libs/utils/WordPressUtils/src/main/java/org/wordpress/android/util/AppLog.java

malinajirka

Thanks @ashiagr! Really great job :). I left to very nitpicky comments, the code looks great. Feel free to merge this PR.

WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/ScanStateListItemBuilder.kt

.../main/java/org/wordpress/android/ui/jetpack/scan/adapters/viewholders/ScanStateViewHolder.kt

Reverted last one due to latest WPAndroidUtils merge conflict

ashiagr · 2020-12-16T09:22:13Z

Thanks for the review @malinajirka ! I've addressed your comments. Ready for another look 🙏.

PS: Don't merge this PR yet. I have to integrate FluxC threats updates and remove temporary FluxC commit hash from this PR.

malinajirka

Thanks for the changes ;)! LGTM

# Conflicts: # build.gradle

ashiagr added 9 commits December 14, 2020 17:08

Introduce threat list item state, scan state button actions

3063d21

Add AppLog.SCAN

906c736

Rename discover list item divider drawable to be reusable

6f04546

Introduce threat list item layout + VH, ScanState list item buttons a…

3ca60f4

…nd introduce scan styles

Introduce ScanStateListItemBuilder

e9e440e

VM.UiState.Content -> add ScanState item from the builder, ThreatItem…

25bd039

… if threats available

Get site's threats from ThreatStore

5a55e2d

Add TODO

f3ffb2f

Add tests for ScanViewModel, ScanStateListItemBuilder

dfb1dce

ashiagr added Jetpack [Status] Not Ready for Merge labels Dec 15, 2020

ashiagr added this to the 16.5 milestone Dec 15, 2020

ashiagr requested a review from malinajirka December 15, 2020 12:28

ashiagr self-assigned this Dec 15, 2020

This was referenced Dec 15, 2020

Jetpack Mobile: Scan Introduce UiState #13578

Merged

Jetpack Section: Scan #13326

Closed

malinajirka self-assigned this Dec 16, 2020

malinajirka approved these changes Dec 16, 2020

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/ScanStateListItemBuilder.kt Outdated Show resolved Hide resolved

.../main/java/org/wordpress/android/ui/jetpack/scan/adapters/viewholders/ScanStateViewHolder.kt Outdated Show resolved Hide resolved

ashiagr added 4 commits December 16, 2020 12:24

Add TODO for adding AppLog.T.SCAN

a91c6a6

Reverted last one due to latest WPAndroidUtils merge conflict

Drop the "action?.let"

9cac9e5

Fix merge

4ca6423

Use dateProvider injectable DateProvider

1769684

malinajirka approved these changes Dec 17, 2020

View reviewed changes

Base automatically changed from issue/13326-scan-introduce-ui-states to develop December 17, 2020 10:27

ashiagr changed the title ~~Issue/13326 Scan UiStates part(ii)~~ Jetpack Mobile: Scan UiStates part(ii) Dec 17, 2020

ashiagr added 4 commits December 18, 2020 12:56

Update fluxc commit hash

ea17787

Remove ThreatStore

712fd19

Fix test

05b6ff8

Merge branch 'develop' into issue/13326-scan-ui-states-partii

ea8fb81

# Conflicts: # build.gradle

ashiagr merged commit c873bfd into develop Dec 18, 2020

ashiagr deleted the issue/13326-scan-ui-states-partii branch December 18, 2020 08:36

ashiagr removed the [Status] Not Ready for Merge label Dec 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jetpack Mobile: Scan UiStates part(ii)#13605

Jetpack Mobile: Scan UiStates part(ii)#13605
ashiagr merged 18 commits intodevelopfrom
issue/13326-scan-ui-states-partii

ashiagr commented Dec 15, 2020 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Dec 15, 2020

Uh oh!

peril-wordpress-mobile bot commented Dec 15, 2020 •

edited

Loading

Uh oh!

peril-wordpress-mobile bot commented Dec 15, 2020 •

edited

Loading

Uh oh!

malinajirka left a comment

Uh oh!

Uh oh!

Uh oh!

ashiagr commented Dec 16, 2020

Uh oh!

malinajirka left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ashiagr commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

To Test

Notes

Merge Instructions

Uh oh!

peril-wordpress-mobile bot commented Dec 15, 2020

Uh oh!

peril-wordpress-mobile bot commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Dec 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

malinajirka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ashiagr commented Dec 16, 2020

Uh oh!

malinajirka left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ashiagr commented Dec 15, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Dec 15, 2020 •

edited

Loading

peril-wordpress-mobile bot commented Dec 15, 2020 •

edited

Loading