-
Notifications
You must be signed in to change notification settings - Fork 799
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
Protect: Integrate ScanReport #40420
Protect: Integrate ScanReport #40420
Conversation
#40057) Co-authored-by: Nate Weller <[email protected]>
* Update Scan and History section header structure/content * changelog * Update projects/plugins/protect/src/js/routes/scan/scan-admin-section-hero.tsx Co-authored-by: Nate Weller <[email protected]> --------- Co-authored-by: Nate Weller <[email protected]>
#40057) Co-authored-by: Nate Weller <[email protected]>
* Update Scan and History section header structure/content * changelog * Update projects/plugins/protect/src/js/routes/scan/scan-admin-section-hero.tsx Co-authored-by: Nate Weller <[email protected]> --------- Co-authored-by: Nate Weller <[email protected]>
bc78984
to
941941d
Compare
projects/js-packages/components/components/scan-report/index.tsx
Outdated
Show resolved
Hide resolved
projects/js-packages/components/components/threats-data-views/index.tsx
Outdated
Show resolved
Hide resolved
projects/js-packages/components/components/scan-report/index.tsx
Outdated
Show resolved
Hide resolved
projects/plugins/protect/src/js/routes/firewall/firewall-statcards.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple small comments and questions, but looks great 👍
On a new site, when starting for free, there are a couple empty line items in the table prior to the scan status loading. Was too fast to grab a screenshot 😛 |
The scan tab uses negative margins to align the DataViews table flush with the edge of the screen (code). Should we do the same here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Left minor comments that could be follow-ups if relevant.
I am wondering if this might fall in with nicely being addressed with this task? We should probably remain consistent with whatever approach we take there - show prior results or hide the table when we have nothing to show? Or is this a separate issue entirely?
This was as per designs, although I am open to the change. Only conflict is that we currently reserve the red icon for the error state in the scan StatCard. Disabled version field sorting and added negative margins! |
Description
Adds
ScanReport
to the Home page in ProtectOther information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
Screenshots