Open zutigrm opened 1 week ago
@zutigrm, could you please update AC to be acceptance criteria without implementation details? Right now there are too many details that affect decisions that IB writer can make which is not something that we should do in AC. Ideally, this AC should explain which groups should be added and which widgets should be in each group. No need to mention which javascript file should be changed.
@eugene-manuilov Thanks, I updated AC. This issue is chained to few other issues in the dependencies, as they are split in few smaller steps, to avoid having one big issue. So in a way steps outlined in design doc need to come together to complete the implementation so I put a bit more details initially. I tried to emphasis on this aspect instead in edited AC.
Let me know what you think.
Thanks, @zutigrm. AC looks better now, but instead of pointing to the document we need to clearly define which groups should be added and which widgets each group should contain because this is our requirement for this task. You can also reference the design doc in AC to let an IB writer where to find explained details of how it should be done.
@eugene-manuilov aha got it, thanks. AC expanded to list the groups and metrics
and
Current selection
- this is a special group, not assigned to any specific widget
Thanks, @zutigrm. AC almost looks good, the only question whether we really need to define the "current selection" group. This is not actually a group, this is a list of currently selected widgets that will be needed only in the ChipTabGroup
component. I think we don't really need to define it in the scope of this task and leave it to be implemented within that component. So let's get rid of it from AC, after that AC will look good.
@eugene-manuilov You are right, AC updated. Thanks
Thanks. AC ✔️
Feature Description
As part of the key metrics selection panel redesign, all items will be belonging to the respective
group
. As part of this issueassets/js/components/KeyMetrics/constants.js
should be updated to include the list of these groups.key-metrics-widgets
should be extended for each metric so it include newmetadata
object as a property, which will holdgroup
property for now. This group should be extracted inMetricItems
and then passed further for filtering where needed.Relevant figma design can be found here
Check the New Metric Grouping Logic Within Sidebar/Selection Panel - Tabbed Functionality section of the design doc, together with it's sub-sections
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Visitors
,Driving traffic
,Generating leads
,Selling products
,Content performance
Visitors
Driving traffic
Generating leads
Selling products
Content performance
Implementation Brief
assets/js/components/KeyMetrics/constants.js
SLUG
andLABEL
, for example forDriving traffic
group -{SLUG: 'drivingTraffic', LABEL: 'Driving traffic'}
assets/js/components/KeyMetrics/key-metrics-widgets.js
metadata
. It should have agroup
property with correct group constant assigned, for examplemetadata: { group: DRIVING_TRAFFIC.SLUG }
metricsListReducer
helper function, so thatgroup
is properly extracted and retuned in the existing object together withtitle
anddescription
https://github.com/google/site-kit-wp/blob/4ef4ed7e31037a90608b534ec8a47a21ced07d7f/assets/js/components/KeyMetrics/MetricsSelectionPanel/MetricItems.js#L57-L79Test Coverage
QA Brief
Changelog entry