Jetpack Mobile: Scan - ThreatItem#13646
Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
|
You can test the changes on this Pull Request by downloading the APK here. |
malinajirka
left a comment
There was a problem hiding this comment.
Thanks @ashiagr! Great job on exploring the web implementation. It looks great overall. I left one important comment and a couple of minor comments.
Some issues I found - not related to this PR, but I thought I'd better mention them so we can add them to backlog, if they are not already there.
Issue 1:
First time I open "Scan" menu item after turning on the flag, the screen content is blank (I can see just AppBar title).
Issue 2:
- Open scan feature for SiteA
- Switch to SiteB
- Open scan feature for SiteB -> state for SiteA appears for 1-2 seconds
WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/builders/ThreatItemBuilder.kt
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/builders/ThreatItemBuilder.kt
Show resolved
Hide resolved
WordPress/src/main/java/org/wordpress/android/ui/jetpack/scan/builders/ThreatItemBuilder.kt
Show resolved
Hide resolved
WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/ThreatItemBuilderTest.kt
Outdated
Show resolved
Hide resolved
WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/ThreatItemBuilderTest.kt
Outdated
Show resolved
Hide resolved
WordPress/src/test/java/org/wordpress/android/ui/jetpack/scan/ThreatItemBuilderTest.kt
Outdated
Show resolved
Hide resolved
|
Thanks for the review @malinajirka! I've addressed your comments and added these issues in the backlog #13326 under known issues. Ready for another round 🙇♀️. PS> I'll assign @zwarm as reviewer tomm., enjoy your 🎄 vacations! |
malinajirka
left a comment
There was a problem hiding this comment.
LGTM, thanks for the changes @ashiagr ;)
Parent: #13326
This PR builds threat item state with header + sub header based on different threat types and click listener that will take it to the threat details screen later on.
To test
Prerequisite: Make sure both Scan feature flag and MySiteImprovements flag are set to on in the App settings.
(Tests included in
ThreatItemBuilderTest)References: Calypso's header & sub header( 1, 2)
Note
FileThreatModel'sSubHeaderbased on threat status (current/fixed/ignored)PR submission checklist:
RELEASE-NOTES.txtif necessary.