Create `ChipTabGroup` component
Feature Description
New ChipTabGroup component should be implemented, to support the selection panel layout updates. Said component should handle group switching, and styles, but it will not contain the implementation of full metrics selection and lists. That will be handled in separate issue.
As part of this issue assets/js/components/SelectionPanel/SelectionPanelItems.js should be used as a starting point for creating a key metrics specific SelectionPanelItems which will be rendering the ChipTabGroup component instead of the current markdown. A new story for the said SelectionPanelItems should be added, to visually test and develop new component. It shouldn't replace the usage of generic SelectionPanelItems in this issue.
Basic group filtering should be implemented as part of this issue
Check New Metric Grouping Logic Within Sidebar/Selection Panel - Tabbed Functionality section and more particularly Implementation of tabbed containers and tab functionality sub-section of the design doc
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- New
ChipTabGroupcomponent is created - It should consist of the new group chips, and tab panel which will list the KMW items
- The chips list all groups implemented in 9376
- Tab panel shows a list of key metric items from the currently active/selected group chip
- Clicking on the inactive group chip should switch/replace the current metric list, by showing only the metric items from the selected group chip.
- Only 1 group chip can be active at the time (shouldn't be possible to select more)
Current selectiongroup chip will be listing the selected items, functionality will be expanded on in 9385 , so for now it can display an empty list.- It should match figma designs
- New
SelectionPanelItemscomponent is also added to render theChipTabGroupcomponent - Refer to the New Metric Grouping Logic Within Sidebar/Selection Panel - Tabbed Functionality section and more particularly
Implementation of tabbed containers and tab functionalitysub-section of the design doc - As part of this issue only
ChipTabGroupcomponent within new key metrics specific selection panel items is added, together with the stories, excluding "new" badge implementation. It should support group chip switching only (filtering all KMW items by the active group), rest of the functionality, and implementation of new panel will come in later issue
Implementation Brief
- Follow the figma design while implementing the following components
Add assets/js/components/KeyMetrics/ChipTabGroup/Chip.js
- It should accept following props:
slugandlabel, which will be extracted from the group object -{SLUG: '...', LABEL: '...'}isActive, a boolean valueselectedCount, number of selected metric items from the current group, if any.onClick, a callback function. It should accept group slug as an argument, so it can switch the active group in the parent's component state
- Add
Buttoncomponent with classgooglesitekit-chip-tab-group__chip-itemfor example - Use the
labelprop for label. - Include
selectedCountvalue astrailingIconprop. You can wrap it inspanand pass the value within parentheses() - For
onClickprop, pass theonClickcallback prop, including theslugprop as an argument - If passed
isActiveprop matches the currentslug, render an extra class in the button, for examplegooglesitekit-chip-tab-group__chip-item--active - Add
assets/sass/components/key-metrics/_googlesitekit-chip-tab-group.scssto include needed styling for bothChipandTabGroupcomponents
Add assets/js/components/KeyMetrics/ChipTabGroup/TabGroup.js
- It should accept following prop:
metricItems- an array of metric item objects to render. This component is not concerned with filtering, and active group selection logic, it will only receive final list that should be output.
- Wrap the markup with main
div.googlesitekit-chip-tab-group__tab-itemfor example - Iterate over
metricItemsand renderMetricItemcomponent for each item, passing the needed props
Add assets/js/components/KeyMetrics/ChipTabGroup/index.js
- It should accept following prop for now (rest of the logic will be implemented in 9385):
allMetricItems- will hold the array of metric objects retrieved fromKEY_METRICS_WIDGETSlist
- Define a new group variable for
Current selection, following the same structure how other existing groups are defined, for example{SLUG: 'currentSelection', LABEL: 'Current selection'} - Define a local state that will be used to update active group, and
isActivevalue will be passed to theChipcomponent. By defaultCurrent selectionslug should be used as initial value. - Define a callback which will be passed to the
Chipcomponent asonClickprop. It should accept a group slug as argument and update the localisActivestate - Filter the
allMetricItemsprop by thegroupproperty, only the items which fulfil this criteriaitem.group === isActiveshould be returned, and assigned to a new variable, sayactiveMetricItems- The resulting
activeMetricItemswill be passed to theTabGroupcomponent as themetricItemsprop
- The resulting
- Define
selectedCountvariable, for now assign it value0- this functionality will be implemented as part of the 9385 - Wrap the markup in main
divwith classgooglesitekit-chip-tab-groupfor example - Collect all ACR groups added in 9376, with
Current selectionbeing the first item in the array, and other groups should follow the order from the design.- Iterate over this list of groups, and render
Chipcomponent for each item, using the current group asgroupprop, and other props previously defined above
- Iterate over this list of groups, and render
Add assets/js/components/KeyMetrics/MetricsSelectionPanel/SelectionPanelItems.js
- Use
assets/js/components/SelectionPanel/SelectionPanelItems.jsas a starting point - It should be key metrics specific component, and render new
ChipTabGroupcomponent instead of the current markup.- Use
KEY_METRICS_WIDGETSfor theallMetricItemsprop
- Use
Test Coverage
- Add stories file, and include it in VRT for the new
assets/js/components/SelectionPanel/SelectionPanelItems.jscomponent, which should render the new selection panel layout
QA Brief
Changelog entry
@zutigrm, could you please remove all technical details from AC and leave only requirements that the new component should match.
@eugene-manuilov AC updated. Thanks
Great! Thanks, @zutigrm. AC ✔️
Thanks, @zutigrm. Mostly looks good, but let's rename active to be isActive and instead of using the group property, let's use slug and label props.
Also, the estimate seems to be very high, can you break down the estimate in a comment to have each component an estimate? Something like this:
- assets/js/components/KeyMetrics/ChipTabGroup/Chip.js - X hrs
- assets/js/components/KeyMetrics/ChipTabGroup/TabGroup.js - Y hrs
- etc
@eugene-manuilov
let's rename active to be isActive and instead of using the group property, let's use slug and label props.
Updated, thanks.
can you break down the estimate in a comment to have each component an estimate?
assets/js/components/KeyMetrics/ChipTabGroup/Chip.js - 2h
assets/js/components/KeyMetrics/ChipTabGroup/TabGroup.js - 1h
assets/js/components/KeyMetrics/ChipTabGroup/index.js - 3.5h
assets/js/components/KeyMetrics/MetricsSelectionPanel/SelectionPanelItems.js - 1h
And stories, probably around 1h
Thanks, @zutigrm. IB looks good and 11 seems to be more reasonable for this component.
QA Update ⚠
Tested this in storybook and here are my observations:
ITEM 1: Comparing with the chips list implemented in #9376 , some are not in the same order. Are we expecting to mirror the exact order? Also under 'Content performance', currently we have the item 'Top earning pages' but in #9376, it's supposed to be 'Top earning content'
ITEM 2: Should we be implementing the dots at the top of groups? Image attached for reference.
ITEM 3:
The AC references the following:
New SelectionPanelItems component is also added to render the ChipTabGroup component
What exactly are these and what should we be looking out for here?
ITEM 4: The current checkbox size is around 14 x 14. Based on Figma, it's 24 x 24.
Figma:
ITEM 5: Based on figma, the checkboxes should be almost vertically centrally aligned in a section. Currently, it's not centrally aligned.
Figma:
ITEM 6: On mobile, the groups are supposed to be in a single line with a sliding user interaction. Currently, the groups appear on 3 lines.
It should be a single line:
ITEM 7: The select text at the bottom should be 14px instead of 12px.
ITEM 8: On mobile, the text 'Edit your personalized goals..etc.' should be 12px instead of 14px.
ITEM 9:
On mobile, the groups should be aligned to start on the same line as the top text. Image is attached for reference. The groups should start at the red line.
ITEM 10: Sometimes when an item is selected, the text 'The metrics you selected require more data tracking. You will be directed to update your Analytics property after saving your selection.' will appear.
Nothing wrong with that but the position looks weird. It sits just below the different items and when you change groups, it moves around.
I think it's a better experience if it sits on top of the bottom section.
https://github.com/user-attachments/assets/03e87e39-754d-42c9-8a42-ec29a1c4274b
ITEM 11:
Font weight for 'Settings' is currently 600 but based on figma, it's 400.
Hi @kelvinballoo thanks for your observations,
ITEM 2: Should we be implementing the dots at the top of groups? Image attached for reference.
This will be implemented in the separate ticket that is already planned for new badge and dots
ITEM 3: The AC references the following: New SelectionPanelItems component is also added to render the ChipTabGroup component What exactly are these and what should we be looking out for here?
This is code related, nothing visual yet
ITEM 1: Comparing with the chips list implemented in https://github.com/google/site-kit-wp/issues/9376 , some are not in the same order. Are we expecting to mirror the exact order? Also under 'Content performance', currently we have the item 'Top earning pages' but in https://github.com/google/site-kit-wp/issues/9376, it's supposed to be 'Top earning content'
We do not expect to be in the same order, they will be computed and listed in the code with same priority. It seems the KMW content is changed to 'Top earning content', we can update it to match.
Items 4-11, and 'Top earning content' this will be finalised in the current issue I am working on #9385 which extends this ticket and replaces the selection panel which will be available for proper testing directly from the dashboard (except for new badge and dots which will be done in #9386). CUrrent issue is more about ChipTab Component and laying down the core infrastructure
QA Update ✅
Thanks @zutigrm , since all the required issues will be fixed under #9385, I will verify those there. I have already put a note in the QAB section for that.
As for this ticket, verifying the storybook, the following were concluded per the QAB:
-
The implementation matches the Figma design 90%. The minor issues that do not match have been documented in this ticket will be verified under #9385
-
Switching between groups shows different metrics belonging to that group
https://github.com/user-attachments/assets/8082e7bb-e7e9-42f1-9102-eacdcb2df320 -
AC points have also been verified and are as expected.
Moving ticket to approval.