Closed felixarntz closed 4 years ago
@felixarntz @aaemnnosttv made a pass at the IB here. Let me know if you've got changes or if it needs a higher level of detail.
@jqlee85 This looks very solid, IB ✅
@jqlee85 This is now unblocked as #1662 has been merged to develop
.
Tested
Installed release candidate zip, activated SK and PSI
Noticed:
Passed desktop view and mobile view.
Sending back to CR for review
@cole10up thanks for the testing. Regarding the those points:
@cole10up - to follow up on what @jqlee85 said
After activation and setup, defaults to mobile metrics. Should we default to desktop metrics?
The default should be mobile. I thought this was in the ACs but apparently it is only in the IB for https://github.com/google/site-kit-wp/issues/1636 which is the issue that handled all of the behavior for the component (this one is just about matching the UI/design).
Regarding the other two points, @jqlee85 is correct.
There is a typo to fix that @jqlee85 found, but if you want to look at the copy in Figma, the design is slightly out of sync and you'll need to reference the comments for the current.
Retested
While testing, noticed setting up PSI from the settings page, the user is directed to the dashboard where the page jumps to an odd spot.
Sending back to CR
Retested
Verified setup auto scroll no longer occurs:
Checked functionality, typo fix, uninstalling and reinstalling.
Passed QA ✅
Feature Description
As a follow-up to #1636, this issue is about implementing the exact UI from the mock.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
Update the a newly created component from #1636 called
DashboardPageSpeed
atassets/js/modules/pagespeed-insights/dashboard/dashboard-widget-web-vitals.js
. The component should:<LabReportMetrics>
and<FieldReportMetrics>
<Layout>
component with noheader
prop to wrap:<header>
(see below)<section>
which wraps<LabReportMetrics>
or<FieldReportMetrics>
(conditionally rendered based on the data source)Create a
<header>
inside<Layout>
which should:@material/react-tab-bar
to switch between the vitals for each scenario utilizing callbacks withsetDataSrcField
andsetDataSrcLab
DeviceSizeTabBar
(see below) which will trigger callbacks to call:setStrategyMobile
andsetStrategyDesktop
The
LabReportMetrics
component should:reportData
as a prop.<div>
rows:<table>
, styled to look the same as Rows 1 & 5 (this should have an invisible<th>
which indiates the types of data):Total Blocking Time
metric (for "In the Lab" only)Largest Contentful Paint
metric (both)Cumulative Layout Shift
metric (both)The
FieldReportMetrics
component should:reportData
as a prop.<div>
rows:<table>
, styled to look the same as Rows 1 & 5 (this should have an invisible<th>
which indiates the types of data):First Input Delay
metricLargest Contentful Paint
metricCumulative Layout Shift
metricCreate a re-usable component called
DeviceSizeTabBar
(open to better names for this) inassets/js/components/device-size-tab-bar.js
which should:setStrategyMobile
andsetStrategyDesktop
).Note: We'll save implementation of the tooltip by the "Speed" module title for another issue.
QA Brief
Changelog entry