Skip to content

Issue/13326 update tabs#13827

Merged
malinajirka merged 12 commits intodevelopfrom
issue/13326-update-tabs
Jan 21, 2021
Merged

Issue/13326 update tabs#13827
malinajirka merged 12 commits intodevelopfrom
issue/13326-update-tabs

Conversation

@malinajirka
Copy link
Copy Markdown
Contributor

@malinajirka malinajirka commented Jan 20, 2021

Parent issue #13326

This PR updates labels of tabs on Scan History screen and implements filtering by ThreatStatus type.

Merge instructions:

  1. Make sure Update subheader and icon for threats in fixed and ignored status #13824 is merged
  2. Remove Not Ready for Merge label
  3. Merge this PR

To test:

  1. Make sure Scan feature flag is enabled
  2. Click on Scan menu item on MySite screen
  3. Click on History action in the toolbar
  4. Notice labels of the tabs are "ALL, FIXED, IGNORED"
  5. Wait until a request finishes (there is no progress and it sometime takes 30s)
  6. Verify the filtering works

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.

@malinajirka malinajirka added this to the 16.6 milestone Jan 20, 2021
@malinajirka malinajirka requested a review from a team January 20, 2021 14:05
@malinajirka malinajirka self-assigned this Jan 20, 2021
@malinajirka malinajirka requested review from ParaskP7 and removed request for a team January 20, 2021 14:05
@peril-wordpress-mobile
Copy link
Copy Markdown

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

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

Base automatically changed from issue/13326-ignored-fixed-threat-status-ui to develop January 21, 2021 08:32
ParaskP7
ParaskP7 approved these changes Jan 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @malinajirka !

I have reviewed and tested this PR as per the instruction, good job!

I have left only minor (🔍) comments and one idea (💡 ). I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.

PS: It seems that there is a CI issue with Lint.

@malinajirka
Copy link
Copy Markdown
Contributor Author

malinajirka commented Jan 21, 2021 •
edited
Loading

Thanks for the review @ParaskP7! I've implemented the suggested changes. Let me know what you think ;).

@malinajirka malinajirka requested a review from ParaskP7 January 21, 2021 10:01
ParaskP7
ParaskP7 approved these changes Jan 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@ParaskP7 ParaskP7 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for applying my suggestions @malinajirka ! 🌟

I have checked the updated and it all looks good to me. Feel free to merge this PR when CI completes.

@malinajirka malinajirka merged commit 040fa3a into develop Jan 21, 2021
@malinajirka malinajirka deleted the issue/13326-update-tabs branch January 21, 2021 10:22
@malinajirka malinajirka mentioned this pull request Jan 21, 2021
Closed
67 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ParaskP7 ParaskP7 ParaskP7 approved these changes

Assignees

@malinajirka malinajirka

Labels

None yet

Projects

None yet

Milestone

16.6

Development

Successfully merging this pull request may close these issues.

2 participants

@malinajirka @ParaskP7