google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.23k stars 286 forks source link

Implement `<PublicationCreate>` component #8836

Closed nfmohit closed 2 months ago

nfmohit commented 4 months ago

Feature Description

A <PublicationCreate> component should be added to the Reader Revenue Manager module that encapsulates the publication creation flow.

Screenshots for reference ![image](https://github.com/google/site-kit-wp/assets/20284937/57f1baf5-daa0-4415-a751-c8ffb9cafb14) ![image](https://github.com/google/site-kit-wp/assets/20284937/90bf798e-20d8-46ab-956e-a112887c51cd)

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

PublicationCreate component

In the assets/js/module/reader-revenue-manager/components/common directory, create a new component, PublicationCreate.js:

Test Coverage

QA Brief

Changelog entry

nfmohit commented 3 months ago

Nice work, @hussain-t ! IB LGTM 👍 ✅

techanvil commented 2 months ago

Hey @hussain-t, the Storybook links in the QAB were pointing to the wrong PR, so I've fixed them.

However - once the PR is merged, the Storybook deployment for the PR will be deleted, so the QAB won't be so useful when it actually comes to QA. I'm not sure if you meant to link to the develop Storybook, or to both the PR and develop versions. Anyhow, please can you update the QAB accordingly?

hussain-t commented 2 months ago

Thanks for pointing it out, @techanvil. I meant to point the links to develop. I've updated the QAB.

kelvinballoo commented 2 months ago

QA Update ⚠️

2 items to check:

  1. The links in the QAB (e.g. https://google.github.io/site-kit-wp/storybook/pull/9088/?path=/story/modules-readerrevenuemanager-setup-publicationcreate--without-publication) is not workable. I have been looking at https://google.github.io/site-kit-wp/storybook/develop/?path=/story/modules-readerrevenuemanager-setup-publicationcreate--without-publication instead. I believe that's the correct one but LMK if otherwise.

  2. Main flag is that the CTA font weight is currently at 400 when it should be 500 from Figma.

    Figma: 500 Screenshot 2024-08-01 at 17 52 30 Implementation: 400 Screenshot 2024-08-01 at 17 47 47
hussain-t commented 2 months ago

Hi @kelvinballoo, sorry about it. Yes, you have spotted the correct URL.

As for the CTA font size, we should keep it consistent with all the buttons. Please take a look at this comment. And the Figma discussions.

kelvinballoo commented 2 months ago

QA Update ✅

Thanks @hussain-t . Looks like it will be fixed under https://github.com/google/site-kit-wp/issues/8856

This ticket can be moved to approval then as the following have been verified as good: âœ