Skip to content

Issue/13326 threat details & history design review#13950

Merged
ashiagr merged 12 commits intodevelopfrom
issue/13326-threat-details-design-review
Feb 5, 2021
Merged

Issue/13326 threat details & history design review#13950
ashiagr merged 12 commits intodevelopfrom
issue/13326-threat-details-design-review

Conversation

@ashiagr
Copy link
Copy Markdown
Contributor

@ashiagr ashiagr commented Feb 3, 2021 •
edited
Loading

Parent #13326

Threat Details & History Screens Design Review PR

Threat Details

Current Threat Fixed Threat Ignored Threat Dark Mode
light mode fixed ignored dark_mode

History

Loading Fixed Ignored Error Dark Mode
history_loading history_fixed history_ignored history_something_went_wrong history_dark_mode

Testing

Prerequisite:

  • Make sure both Scan feature flag and MySiteImprovements flag are set to on in the App settings.
  • You have access to WP.com account with a site having scan capability and multiple threats (e.g. http://do.wpmt.co/jp-complete).

PS - Jetpack Threat Tester (Internal ref: pbuNQi-Kb) can be used to re-add threats once they're fixed.

Merge Instructions:

  1. Make sure PR Issue/13326 scan design review #13949 is merged to develop
  2. Remove "Not Ready for Merge" label
  3. Merge this PR

Notes

  • "DiffViewer" for a core file threat is not implemented.
  • Tweaks for tablet - not included in this MVP release.
  • History screen does not include date headers.

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.

@ashiagr ashiagr added [Status] Needs Design Review A designer needs to sign off on the implemented design. [Status] Not Ready for Merge labels Feb 3, 2021
@ashiagr ashiagr added this to the 16.7 milestone Feb 3, 2021
@ashiagr ashiagr self-assigned this Feb 3, 2021
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Feb 3, 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 Feb 3, 2021 •
edited
Loading

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

@ashiagr ashiagr changed the title Issue/13326 threat details design review Issue/13326 threat details & history design review Feb 3, 2021
@ashiagr ashiagr requested a review from zwarm February 3, 2021 13:57
@zwarm zwarm self-assigned this Feb 3, 2021
zwarm
zwarm approved these changes Feb 3, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

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

Looking good @ashiagr 👍 from me

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

osullivanchris commented Feb 3, 2021

👋 Hey thanks again for the PR. Nearly all the UI issues are covered in #13949 so I don't have much to add here.

The only one specific to this PR is the blocks of code. I don't think I provided you with an Android mock of that so its my fault. But I'm thinking we should use system colours for the greys so they automatically work in dark mode as well. And then I can provide red colour values for light mode, and another set of red colour values for dark mode. Does that work for you?

The only reason I am not sending them up-front is its sometimes hard to know what set of colours we are using in the code as it doesn't always match what I have in design tools. Lets sync up on it tomorrow.

@ashiagr ashiagr mentioned this pull request Feb 4, 2021
Closed
67 tasks
Base automatically changed from issue/13326-scan-design-review to develop February 4, 2021 15:26
@ashiagr
Copy link
Copy Markdown
Contributor Author

ashiagr commented Feb 5, 2021 •
edited
Loading

👋 Chris,

Summarising our discussion on Slack, we applied these colors based on color suggestions in #13949 (comment) + utilising some of the existing colors with emphasis values for the gray shades.

new_colors

@osullivanchris
Copy link
Copy Markdown

osullivanchris commented Feb 5, 2021

Summarising our discussion on Slack, we applied these colors based on color suggestions in #13949 (comment) + utilising some of the existing colors with emphasis values for the gray shades.

Looks good, thanks @ashiagr !

@ashiagr ashiagr merged commit 9382fed into develop Feb 5, 2021
@ashiagr ashiagr deleted the issue/13326-threat-details-design-review branch February 5, 2021 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zwarm zwarm zwarm approved these changes

@osullivanchris osullivanchris Awaiting requested review from osullivanchris

Assignees

@zwarm zwarm

@ashiagr ashiagr

Labels

[Status] Needs Design Review A designer needs to sign off on the implemented design.

Projects

None yet

Milestone

16.7

Development

Successfully merging this pull request may close these issues.

3 participants

@ashiagr @osullivanchris @zwarm