Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide recommendations when Site Scan results are stale #6690

Closed
westonruter opened this issue Nov 5, 2021 · 5 comments · Fixed by #6696
Closed

Hide recommendations when Site Scan results are stale #6690

westonruter opened this issue Nov 5, 2021 · 5 comments · Fixed by #6696
Assignees
Labels
Changelogged Whether the issue/PR has been added to release notes. Enhancement New feature or improvement of an existing one P0 High priority Site Scanning
Milestone

Comments

@westonruter
Copy link
Member

Feature Description

If the Site Scan results are stale or the Site Scan is currently in progress, none of the Template Modes should have “Recommended” or “Not recommended” appear anywhere. These should only be shown if the Site Scan is complete and the results are not stale.

Additionally, it may make sense to hide the notice above the template modes since the Site Scan box already is indicating "Stale Results":

image

Acceptance Criteria

No response

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

@westonruter westonruter added Enhancement New feature or improvement of an existing one P0 High priority labels Nov 5, 2021
@westonruter westonruter added this to the v2.2 milestone Nov 5, 2021
@delawski
Copy link
Collaborator

delawski commented Nov 8, 2021

The Template Mode recommendations (both on the AMP Settings screen and in the Wizard) are defined in getTemplateModeRecommendation() and that's where the logic needs to be updated. However, since the descriptions and the recommendation levels are based on the matrix in the Template Mode Selection doc, it's worth first updating the matrix and use it as a single source of truth.

The notice about the results being stale it not related to getTemplateModeRecommendation() and should be removed separately.

@dhaval-parekh
Copy link
Collaborator

✅ Remove notice about stale scan results from the Template Mode section on the AMP Settings screen.

✅ Do not show any recommendations when the scan results are stale.

⚠️ Do not show any recommendations when a scan is in progress.
When results are not stale but the user still rescans the site. recommendations are still displayed to the user

AMP-GH-6690.mov

@westonruter
Copy link
Member Author

⚠️ Do not show any recommendations when a scan is in progress.
When results are not stale but the user still rescans the site. recommendations are still displayed to the user

If the results are not stale, I don't think an in-progress scan should necessarily hide the recommendations. If the results are stale then certainly the recommendations should be hidden, both before and during a scan. @delawski Isn't that right?

@dhaval-parekh
Copy link
Collaborator

dhaval-parekh commented Dec 2, 2021

If the results are not stale, I don't think an in-progress scan should necessarily hide the recommendations.

I agree with that.

If the results are stale then certainly the recommendations should be hidden, both before and during a scan.

Yes, It is hidden when results are stale and during the scan (if results are stale).

With that can mark this issue as QA Passed.

@delawski
Copy link
Collaborator

delawski commented Dec 2, 2021

If the results are not stale, I don't think an in-progress scan should necessarily hide the recommendations. If the results are stale then certainly the recommendations should be hidden, both before and during a scan. @delawski Isn't that right?

That is correct 👍

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Enhancement New feature or improvement of an existing one P0 High priority Site Scanning
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants