Skip to content

Jetpack Section: Threat Details screen#15720

Merged
momo-ozawa merged 19 commits intodevelopfrom
task/15190-threat-details
Jan 29, 2021
Merged

Jetpack Section: Threat Details screen#15720
momo-ozawa merged 19 commits intodevelopfrom
task/15190-threat-details

Conversation

@momo-ozawa
Copy link
Copy Markdown
Contributor

@momo-ozawa momo-ozawa commented Jan 27, 2021

Part of #15190
WPKit: wordpress-mobile/WordPressKit-iOS#337

Description:

  • Added Threat Details screen
  • TODO: hook up buttons
  • TODO: display threat context

To test:

  1. Launch the app, tap on the My Site, tap on a site with Jetpack Scan and also has Threats
  2. Tap on a Threat cell
    • ✅ Should navigate to a new Threat Details screen
    • ✅ Threat Details screen should be scrollable (if the content height is bigger than the screen height)
  3. Launch Calypso and go to Jetpack > Scan, and compare info shown in Threat Details screen to Calypso
    • ✅ Info displayed on the Threat Details screen should match info displayed on Calypso

Example screenshots:

Simulator Screen Shot - iPhone 12 Pro - 2021-01-27 at 15 34 04 Simulator Screen Shot - iPhone 12 Pro - 2021-01-27 at 15 34 09 Simulator Screen Shot - iPhone 12 Pro - 2021-01-27 at 15 33 59

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.

@momo-ozawa momo-ozawa added this to the 16.7 milestone Jan 27, 2021
@momo-ozawa momo-ozawa self-assigned this Jan 27, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

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

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

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Jan 27, 2021

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 27, 2021 •
edited
Loading

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

emilylaguna
emilylaguna suggested changes Jan 27, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

This is looking awesome! I did find a crash and left a few comments below.

  • If I tap on the status cell the app crashes:

Screen Shot 2021-01-27 at 2 46 09 PM

@emilylaguna emilylaguna mentioned this pull request Jan 28, 2021
Closed
55 tasks
emilylaguna
emilylaguna approved these changes Jan 28, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Awesome, left 1 final comment below. Approving now since it's not major.

Momo Ozawa added 3 commits January 29, 2021 09:49
Merge fixanld and unknown strings into default
0fc0740
Merge branch 'develop' into task/15190-threat-details
b7f09e4 
# Conflicts:
#	WordPress/Classes/ViewRelated/Jetpack/Jetpack Scan/JetpackScanViewController.swift
#	WordPress/WordPress.xcodeproj/project.pbxproj
Move row check logic into threat method
c1d7506
@momo-ozawa momo-ozawa merged commit 27667d2 into develop Jan 29, 2021
@momo-ozawa momo-ozawa deleted the task/15190-threat-details branch January 29, 2021 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leandroalonso leandroalonso Awaiting requested review from leandroalonso

1 more reviewer

@emilylaguna emilylaguna emilylaguna approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@momo-ozawa momo-ozawa

Labels

Jetpack Mobile [Type] Task

Projects

None yet

Milestone

16.7

Development

Successfully merging this pull request may close these issues.

2 participants

@momo-ozawa @emilylaguna