Skip to content

Jetpack Section: Scan Status#15606

Merged
emilylaguna merged 22 commits intodevelopfrom
task/15190-scan-status
Jan 11, 2021
Merged

Jetpack Section: Scan Status#15606
emilylaguna merged 22 commits intodevelopfrom
task/15190-scan-status

Conversation

@emilylaguna
Copy link
Copy Markdown
Contributor

@emilylaguna emilylaguna commented Jan 7, 2021

Project: #15190

This adds the initial logic and view states for the scan status view.

No buttons work, and the scanning view doesn't display progress yet.

Idle

No Threats (okay) Has Fixable Threats Has Unfixable Threats Single Threat
no threats multiple fixable threats multiple unfixable threats single unfixable threat

Scanning

Preparing Scanning
preparing scanning

To test:

  1. Launch the app
  2. Tap on the My Site tab
  3. Tap on a site with Jetpack Scan enabled
  4. Tap the Scan item
  5. Note the different states of the view
  6. On the web trigger a scan
  7. Tap back and tap scan again to reload the view
  8. Wait for the scan to start
  9. Tap back and tap scan again to reload the view
  10. Repeat for each state

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.

@emilylaguna emilylaguna added this to the Someday milestone Jan 7, 2021
@emilylaguna emilylaguna self-assigned this Jan 7, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 7, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link
Copy Markdown

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

Generated by 🚫 dangerJS

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 7, 2021

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

Copy link
Copy Markdown
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

@emilylaguna Works as described!
Added a couple of minor suggestions.

Comment on lines +17 to +19
init(blog: Blog, view: JetpackScanView,
service: JetpackScanService? = nil,
context: NSManagedObjectContext = ContextManager.sharedInstance().mainContext) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[Nit]
Indentation alignment seems to be off, and the view: parameter would look better on its own line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oops, thanks for the catch! Fixed in: d0f84cd

Comment on lines +47 to +52
tableView.register(UINib(nibName: String(describing: JetpackScanStatusCell.self), bundle: nil),
forCellReuseIdentifier: Constants.statusCellIdentifier)

tableView.register(UINib(nibName: String(describing: JetpackScanThreatCell.self), bundle: nil),
forCellReuseIdentifier: Constants.threatCellIdentifier)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think of conforming to the NibReusable protocol for the cells?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done: 67315da

case error
}

private static func viewState(for scan: JetpackScan) -> StatusViewState {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! makes it easy to follow how scan states map to view states ✨

// IBOutlets
@IBOutlet weak var tableView: UITableView!

//
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing comment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done: ff07461

@emilylaguna emilylaguna mentioned this pull request Jan 8, 2021
55 tasks
@emilylaguna emilylaguna modified the milestones: Someday, 16.6 Jan 11, 2021
@emilylaguna emilylaguna merged commit 1c3cab4 into develop Jan 11, 2021
@emilylaguna emilylaguna deleted the task/15190-scan-status branch January 11, 2021 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants