Refactor the Key Metrics Selection Panel, extracting components for reuse in the Audience Segmentation Selection Panel. about site-kit-wp

[techanvil]

Hi @ankitrox, thanks for drafting the IB/POC. It's definitely heading in the right direction, however there are a number of aspects I would like to see addressed.

• We should be aiming to reuse the CSS classes more than your POC demonstrates.
• We should also look to share more common logic and structure, while keeping the refactor as simple as we can.
• I think we can avoid the saved flag in the items and we don't need a suffix at this point either.

I figured it would be easiest to show you what I mean in more detail by putting together a POC myself: #8676. I think this is most of the way there and the main thing remaining to do is to genericise the CSS class names.

Please take a look at my POC, consider my feedback, let me know your thoughts if you wish to discuss this, and/or update the IB accordingly.

[techanvil]

Thanks @ankitrox, nice work here so far on the IB.

I've made a few tweaks, to fix a couple of minor errors and remove some duplication, which you can see in the edit history.

Additionally - although I did mention you could take the approach of not listing all the props for each item and instead refer to the POC, where you have listed the props (which is totally fine), you've missed most of them for the SelectionPanelFooter component. Please can you provide the full list of props for this component too, for consistency?

[ankitrox]

Thanks @techanvil for fixing typos and formatting.

I have expanded on SelectionPanelFooter component props as suggested. I haven't provided too much info on MetricsFooter because it would be repetitive.

Assigning you for review.

Thanks again!

[techanvil]

Thanks @ankitrox, that's great. IB LGTM! ✅

[kelvinballoo]

QA Status ⚠️

This is tested good overall:

• CSS classes used by these generic components have been renamed to remove references to Key Metrics. ✅

  

• The KM Selection Panel is unchanged from the user's perspective. Compared prod and develop and the panels are consistent with each other. ✅

• In Storybook, the Components > Selection Panel story variants, their behaviour and styling is aligned with the key metrics selection panel ✅

• Layout, margin, padding, font family , font sizes are consistent with Key Metrics panel ✅

• Verified on Mobile, tablet and other browsers too and results are consistent across. ✅

• The only behaviour that I noticed is different is when clicking a checkbox. Refer to the detailed video below ⚠️

  Checkbox.behaviour.mov

[nfmohit]

Thank you for sharing your observation, @kelvinballoo!

• The only behaviour that I noticed is different is when clicking a checkbox. Refer to the detailed video below ⚠️

Interesting observation. However, as long as the focus behaviour is unchanged in the actual application, we should be good to go here, which I just checked and ensured that it is.

See screen recording

1.mp4

The Storybook story for SelectionPanel uses a custom set of made-up items and relies on the local state to store the selection. The focus gets lost there most likely due to re-rendering events. I believe we can overlook that.

Please let me know if you have any other concerns, thank you, Kelvin!

[kelvinballoo]

QA Updates ✅

Thanks for the explanation @nfmohit . That makes sense.
Moving this ticket to Approval.