Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement settings view screen for RRM #8842

Open
6 tasks
nfmohit opened this issue Jun 8, 2024 · 3 comments
Open
6 tasks

Implement settings view screen for RRM #8842

nfmohit opened this issue Jun 8, 2024 · 3 comments
Labels
Module: RRM Reader Revenue Manager module related issues P0 High priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature

Comments

@nfmohit
Copy link
Collaborator

nfmohit commented Jun 8, 2024

Feature Description

The module settings view screen should be added for the Reader Revenue Manager module. The screen should simply show the ID of the connected publication.

Screenshot for reference

image


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

Acceptance criteria

  • The module settings view screen should be implemented for the Reader Revenue Manager module according to the Figma designs.
  • The screen should display the ID of the connected publication (labelled as "Publication"), followed by a notice that conveys the publication onboarding state (via the <PublicationOnboardingStateNotice> component being implemented by #8838).

Implementation Brief

  • In assets/js/modules/reader-revenue-manager/components/settings/SettingsView.js component added in Add JS entry point for RRM #8786, retrieve the publication ID with useSelect using getPublicationID selector.
  • Return a div container with googlesitekit-setup-module class name which should wrap the following element with the mentioned structure as below.
    • A wrapper div with the class name googlesitekit-settings-module__meta-items and inside it another divwith class googlesitekit-settings-module__meta-item.
    • The setting label should be an h5 with the class name googlesitekit-settings-module__meta-item-type and the setting value should be a p with the class name googlesitekit-settings-module__meta-item-data, that renders a DisplaySetting component with the value as publication ID.
    • A reference of assets/js/modules/analytics-4/components/settings/SettingsView.js can be taken to build in similar way.
  • Add the PublicationOnboardingStateNotice component to the main wrapper div.

Test coverage

  • Add story for the component.

QA Brief

Changelog entry

@nfmohit nfmohit added P0 High priority Type: Enhancement Improvement of an existing feature Team M Issues for Squad 2 Module: RRM Reader Revenue Manager module related issues labels Jun 8, 2024
@ivonac4 ivonac4 added Module: RRM Reader Revenue Manager module related issues and removed Module: RRM Reader Revenue Manager module related issues labels Jun 11, 2024
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jun 14, 2024
@ankitrox ankitrox assigned ankitrox and unassigned ankitrox Jul 3, 2024
@nfmohit nfmohit self-assigned this Jul 4, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jul 6, 2024

Thank you for the IB, @ankitrox ! Please take a look at my comments below:

  • Return a div container with googlesitekit-setup-module class name which should wrap following element.

An additional class name specific to the module would be nice to add, i.e. googlesitekit-setup-module--reader-revenue-manager.

  • A heading Publication and text which should be the publication ID. Note that the heading is smaller than the ID text. Please refer to the figma design. These both elements should be wrapped within a single div.

Using the appropriate class names and components in crucial here. Let's mention using the wrapper div to use the googlesitekit-settings-module__meta-item class, which should also have another wrapper div with the class googlesitekit-settings-module__meta-items.

The setting label should be an h5 with the class name googlesitekit-settings-module__meta-item-type and the setting value should be a p with the class name googlesitekit-settings-module__meta-item-data, that renders a DisplaySetting component with the publication ID.

It may also be beneficial to reference another module's SettingsView component to get an idea of the structure, e.g. assets/js/modules/analytics-4/components/settings/SettingsView.js. Using the appropriate markup is crucial to ensure a consistent structure.


Please let me know if you have any questions, thank you!

@nfmohit nfmohit assigned ankitrox and unassigned nfmohit Jul 6, 2024
@ankitrox
Copy link
Collaborator

ankitrox commented Jul 8, 2024

Thank you @nfmohit . I've updated the IB as per mentioned points.

@ankitrox ankitrox assigned nfmohit and unassigned ankitrox Jul 8, 2024
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jul 9, 2024

Thank you @ankitrox . I've made some minor linguistic improvements to the IB, it looks good to me 👍

IB ✅

@nfmohit nfmohit removed their assignment Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Module: RRM Reader Revenue Manager module related issues P0 High priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants