Create `<DashboardSharingSettings />`

[aaemnnosttv]

Feature Description

This issue is for building out the main interface for dashboard sharing settings which will later be integrated with the dashboard sharing settings button + modal being introduced in #4820. Since the integration is happening at a later stage, these issues can happen in parallel using Storybook for the development environment.

---

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

Acceptance criteria

• A new DashboardSharingSettings component should be created in assets/js/components/dashboard-sharing
• Should use components created in other issues:

  • 4821

  • Sub-components may be created as needed
• The design should follow the relevant portions of the Figma document for dashboard sharing
  • The component should include everything within the contents of the modal (not the modal itself)
  • Any deviation from the design should be noted in the QA brief
  • Attention should be paid to differences between phone/mobile and larger viewports
• The table of Products (modules) should be primarily sorted by modules that the user owns the connection for first, can manage the settings for second, and then by the module's order property
  • The end result should be a table where the modules the user has the most control over are at the top and least/no control over at the bottom
• Note the difference in interface between single-admin and multi-admin environments (see hasMultipleAdmins on the core/site store)
  • Single admin environments have columns for Product, Who can view
  • Multi- admin environments have columns for Product, Who can view, Who can manage view access (column headers may change in the future)

• For modules that a user cannot manage sharing for, the static message should be shown in place of the role select:

  Contact managing user to manage view access

  • For multi-admin environments, the select for a module's sharing management is replaced by additional information about the module's owner where admin-2 is the user's login/username:
    Managed by admin-2 ℹ️

  The ℹ️ indicates the icon + tooltip as shown in the design

• Choices for who can manage view access correspond to the options for a module's sharing management:
  • owner "Only me" (default)
  • all_admins "All Admins"
• Rows for shared ownership modules are special in that these are manageable by all admins – their sharing management setting (not role select) should be disabled
• Rows for other modules become disabled while editing the shared roles for a module
• Applying the sharing settings should work similar to module settings (except there is only an edit view rather than separate view + edit)
  • Submitting can only happen if settings have changed (see haveSharingSettingsChanged())
  • Submitting settings using saveSharingSettings()
  • A spinner should display (not shown in design) while the submit action is in progress and all setting fields should be disabled
• When changes to user roles are made, an informative dialog should be shown in the footer of the settings to inform the user of the the implications of the change
  • A similar but different message is shown if changing a module's settings management to "All admins" which would take precedence over the message about a change in selected roles
• Storybook stories should be added for the component, no integration should be done yet
• URLs for any Learn More links should use https://sitekit.withgoogle.com/documentation/using-site-kit/dashboard-sharing/

Implementation Brief

• Create DashboardSharingSettings component in assets/js/components/dashboard-sharing.

Grouping the modules by owned, manageable and the rest

• Get all the modules from getModules() selector.
• Return null if the modules is not available.
• Create PERMISSION_MANAGE_MODULE_SHARING_OPTIONS constant with the value googlesitekit_manage_module_sharing_options in assets/js/googlesitekit/datastore/user/constants.js.
• To get the owned and manageable modules, loop through the modules object and use the hasCapability() selector by passing the PERMISSION_MANAGE_MODULE_SHARING_OPTIONS permission and the module slug.
• Add sharingManagement property using the getSharingManagement(moduleSlug: string) selector which is being implemented in #4795.
• The rest of the other modules that have the least priority should be sorted by the order property. Note that thegetModules() selector already returns modules sorted by their order.

Creating the layout

• The designs should match the Figma designs for the larger and smaller screens
• Use Grid -> Row -> Cell and each cell having an appropriate lgSize, mdSize, and smSize.
• All the cells beneath the same column will use the same sizes.
• Loop through the manageable modules array to create a grid layout.
• Use the <ModuleIcon /> component and pass the slug to render the module icon.
• Single admin environments have columns for Product, and Who can view.
• Multi-admin environments should have an additional column Who can manage view access.
• Show the multi-admin column if there are multiple admins using the hasMultipleAdmins() selector.
• For modules that a user can manage sharing for, render the <UseRoleSelect /> component.
• When editing the roles, all the setting fields should be disabled.
• For modules that a user cannot manage sharing for, the static message should be shown in place of the role select:

  Contact managing user to manage view access

• For multi-admin environments:
  • the select for a module's sharing management is replaced by additional information about the module's owner where admin-2 is the user's login/username:

    Managed by admin-2 ℹ️

  • Get the module owner's login/username from modules[module].owner.login.
  • Export the ℹ️ from the Figma design
  • Use the <Tooltip /> from material-ui/core that should have the following text where admin-2 is the user's login/username:

    Admin 2 has connected this and given managing permissions to all admins. You can change who can view this on the dashboard.

  • Choices for who can manage view access correspond to the options for a module's sharing management.
  • owner "Only me" (default) and all_admins "All Admins" should be available in the sharingManagement property obtained using the getSharingManagement(moduleSlug: string) selector (as we mentioned in the Grouping the modules).

Dashboard Sharing Settings Footer

• Create DashboardSharingSettingsFooter component in assets/js/components/dashboard-sharing.

• Extract these lines from assets/js/components/dashboard-sharing/DashboardSharingSettingsButton.js to the new component. https://github.com/google/site-kit-wp/blob/a2823b7c40c1e5cd5e398d5699b0a7c9a5ea28e1/assets/js/components/dashboard-sharing/DashboardSharingSettingsButton.js#L131-L139

• Enable the Apply button only if settings have changed. In otherwords, disable the button if the settings haven't changed.

• Check whether the sharing settings have changed using the haveSharingSettingsChanged() selector.

• Render the <Spinner /> component while the submit action is in progress and all setting fields should be disabled

• On submitting the settings should dispatch the saveSharingSettings() action which is being implemented in #4794

• Check how the Confirm Changes button work in assets/js/components/settings/SettingsActiveModule/Footer.js.

• To display the footer notice upon changes:

  • Create a class googlesitekit-sharing-settings-notice or something appropriate. The styles should match the Figma designs.
  • For user role changes the message should be:

    By clicking Apply, you’re granting the selected roles view-only access to data from the Google services you’ve connected via your account.

  • If there is a change in the sharing settings management, that notice should take precedence over the selected role changes (message).
  • For sharing settings management changes the message should be:

    By clicking Apply, you are giving other admins permission to manage view-only access on your behalf to data from the Google services you selected “All Admins” for.

• Add stories for the new <DashboardSharingSettings /> component.

Test Coverage

• Add unit tests for the <DashboardSharingSettings /> component that covers the outlined features in the AC.

QA Brief

• Note the Footer component isn't rendered within the DashboardSharingSettings component because the DashboardSharingSettings has to be children of DialogContent and the Footer has to be children of DialogFooter. Hence the Footer is rendered in the DashboardSharingSettingsButton component.

• Verify the component in Storybook ("Components -> DashboardSharingSettings -> * ") matches the designs in Figma. Only the component itself, not the modal and modal footer.

• Verify the functionality of the component as per the AC.

• Verify the DashboardSharingSettingsButton component in Storybook ("Components -> DashboardSharingSettingsButton -> * ") renders the DashboardSharingSettings component and matches the designs in Figma.

• Verify the trackEvent as per this comment

Changelog entry

• Add settings modal to allow admins to configure Dashboard Sharing.

[felixarntz]

@aaemnnosttv ACs LGTM! Just one thing to iterate (this may apply to other issues as well, so please keep it in mind for other ACs): Per #4949 (which came out of a conversation on another issue), let's always use https://sitekit.withgoogle.com/documentation/using-site-kit/dashboard-sharing/ for all dashboard sharing-related "Learn more" links for now. #4949 will later be implemented to replace all of these links as needed.

[tofumatt]

Thanks @hussain-t! This largely looks good; just a few points in the IB I'm not quite clear on.

The rest of the modules should be in the least priority.

What does this point refer to? I don't quite follow 😅

Use Grid -> Row -> Cell and each cell having a specific class name and set the width in %.

Don't we usually set the width in numbers referring to cell size? Rather than in percentage., shouldn't we do things like <Cell size={ 12 } />?

---

I think we should probably look to add some tests for this as it's a big component and worth testing some of the features outlined, though admittedly that will likely bump up the estimate.

I think largely the rest of the IB looks good, but do you think there are some easy wins for testing here? And could you address the few questions I mentioned above?

After that seems good I think, though this issue's ACs + IBs are a lot to go through 😅

[hussain-t]

Thanks, @tofumatt; as for your questions:

The rest of the modules should be in the least priority. What does this point refer to? I don't quite follow 😅

The AC mentions:

The table of Products (modules) should be primarily sorted by modules that the user owns the connection for first, can manage the settings for second, and then by the module's order property

I meant group/sort the modules by owned/manageable/other. The other modules should be ordered by the order property. The getModules() selector already returns modules sorted by their order. Sorry for the confusion; I have updated the IB.

---

Don't we usually set the width in numbers referring to cell size? Rather than in percentage., shouldn't we do things like <Cell size={ 12 } />?

Thanks, that's correct. I have updated the IB.

---

Regarding adding tests for this component, I thought we could omit them since we will be adding stories. However, I do agree with you that such a big component should have some unit tests for bulletproofing 👍

After that seems good I think, though this issue's ACs + IBs are a lot to go through 😅

💯

[hussain-t]

@tofumatt, we need to bump up the estimate for adding test coverage. However, we don’t have anything between 19 to 30; what should I do?

[tofumatt]

@hussain-t In that case, let's go with 30; the estimate is at the high end of things but better to err on that side of things.

[tofumatt]

Alright, a lot to go through here but I think this all looks good. IB ✅

[asvinb]

Working branch: feature/4822-dashboard-sharing-settings

[hussain-t]

As this ticket is blocking #5221, @eugene-manuilov and I discussed adding the trackEvent call in the DashboardSharingSettings component from #5221 to this ticket. Hence I'm removing #5221 from blocking.

Within file assets/js/components/dashboard-sharing/DashboardSharingSettings.js:

• Add a call to trackEvent to the management permission dropdown change handler.
  • Event name:
  • When the dropdown value is all_admins, the event name should be change_management_all_admins.
  • When the dropdown value is owner, the event name should be change_management_owner.
  • Event category: ${viewContext}_sharing.
  • Event label: the module slug for which the management setting was changed.
• Add a call to trackEvent to the Apply button click handler.
  • Event name: settings_confirm.
  • Event category: ${viewContext}_sharing.
• Add a call to trackEvent to the Cancel button click handler.
  • Event name: settings_cancel.
  • Event category: ${viewContext}_sharing.

[wpdarren]

QA Update: ⚠️

@tofumatt @techanvil I haven't started to QA this yet, but while creating my test plan I came across a console error when clicking on the 'Add roles' link for any of the modules.

Uncaught TypeError: Invalid attempt to spread non-iterable instance. In order to be iterable, non-array objects must have a [Symbol.iterator]() method.

Also, just quickly reading through the QAB, it mentions checking to Storybook, but at this stage, we should only be testing on InstaWP test sites. With this in mind are there any particular things we should be testing? My initial thoughts are every aspect of the Dashboard sharing & permissions functionality needs testing.

[tofumatt]

Also, just quickly reading through the QAB, it mentions checking to Storybook, but at this stage, we should only be testing on InstaWP test sites. With this in mind are there any particular things we should be testing? My initial thoughts are every aspect of the Dashboard sharing & permissions functionality needs testing.

That was part of the QA Brief: to check storybook stories. For what it's worth I've looked through those already and they look good.

In terms of actual functionality, yeah, testing all aspects of Dashboard Sharing for this one—verifying that the changes made in the sharing settings are made for users—would be good. 👍🏻

---

In terms of the JS error, I'm taking a look now, I missed that one. 😓 I'll make a PR if a change is needed.

[tofumatt]

@wpdarren Can you walk me through the steps you took to get that JS error? I wasn't able to recreate it in development.

[tofumatt]

Nevermind, I just noticed the error only happens when no sharing settings are set and the sharing button is clicked. I have a PR ready for this 🙂

[wpdarren]

QA Update: ❌

Note: this was initially posted on #4821 because I was focusing my testing on the user roles selection. @asvinb has asked me to post it here too. I have kept this ticket in the QA queue though so I can continue to work on the testing.

1. I select a role 'Editor' for Search console and Analytics. Click apply, and the Dashboard Sharing settings modal disappears. I click back on the icon and the two modules I added roles to, are blank, 'no rules'.. I also tested this with AdSense connected and the same issue occurs. Only seems to be saving PSI and Idea Hub.

Screencast here:

https://user-images.githubusercontent.com/73545194/171643667-63050c39-82f5-489f-9f4b-05430014f6fa.mp4

2. The 'Learn more' link doesn't have the external link icon.

3. I only had one admin user when I initially added roles to two modules. I then created a secondary admin user and went back to the Dashboard sharing & permissions screen, and the Who can manage view access dropdown is inactive for those modules. They are active for the two modules that weren't previously set. Maybe this is expected but even though these are shareable modules, we should allow the just the connected admin permission?

Screenshot


4. Odd UX/UI in a specific scenario. I go to a module, i.e. Idea Hub, and add any role, and click on the apply button. It's the only change that I want to make so do not click on the X icon to close it. When I go back to Dashboard sharing & permissions screen, the Idea Hub, shows the roles, because I didn't close it before applying. It would be more user friendly, to have the roles close when the apply button is clicked. Here's a screencast.

https://user-images.githubusercontent.com/73545194/171646262-a716a768-60c3-4684-8074-2053eb423243.mp4

Note: the same behaviour appears when you also click on the 'Cancel' link.

5. For the Product and Who can view titles, the font is Google Sans on Figma but is Roberto on my site. The font sizes and color are slightly different too between the site and Figma. This could be expected but did not want to assume.

Screenshot


6. For the module titles and roles, the font is Google Sans on Figma but is Roberto on my site. The font colors are also different when viewing the different texts (module title, role title and 'Add role' text, as per screenshot.

Screenshot


7. The closing of the Dashboard Sharing settings modal when clicking outside is inconsistent. When clicking in some areas I am able to close it, but other areas I am not. Seems that you can close it when clicking above or below the modal but not at the side, not a huge issue, but thought I would mention it as it's likely to come up in bug bash.

https://user-images.githubusercontent.com/73545194/171654543-7a400d84-d51d-4a88-aeff-798269b9e90f.mp4

8. Minor UX/UI on the Desktop Sharing settings icon in the header. When the modal is open, the icon does not have any hover/active background color, which is to be expected. When you click cancel or apply in the modal, it closes as expected but the icon is now highlighted. The colour that the icon is highlighted is a different colour than when you hover over it.

Screenshot


[wpdarren]

QA Update: ❌

On top of the issues reported above I have additional observations.

9. Extending from issue 1, reported above, this also occurs with the selection of Who can manage view access dropdown. In the example here, for Search Console, I select 'All Admin' and click on the apply button. When I click back into the dashboard sharing settings modal, the change has not been saved. No errors in the console. Screencast here

https://user-images.githubusercontent.com/73545194/171674683-3256bf19-c33f-4676-8e2f-0210457e0073.mp4

10. Odd UX/UI on small screen sizes when you are adding roles on a site that has two admin users. The Who can manage view access title appears at the top, but the dropdown appears underneath the user roles.

Screenshot


11. Odd UX/Ui on small screen sizes when you select Who can manage view access dropdown. The layout shifts and the dropbox appears further down the page. Possibly linked with issue 10. above.

Screenshot


12. Minor styling issue on the Managed by tooltip when you have two admin users managing dashboard sharing settings. It slightly overlaps the modal as per screenshot.

Screenshot


13. Should the tooltips appear on small screen sizes like mobiles? I am assuming that they would not function on mobile.

Screenshot


14. This might be linked with issue 3 above. While logged into an admin account, for PSI and Idea Hub module they are managed by another admin user anadmin. When I login as anadmin I am unable to change Who can manage view access as they are inactive dropdowns. See screenshots below Is this expected?

Update: I just spotted this in the AC: Rows for https://github.com/google/site-kit-wp/issues/4791 are special in that these are manageable by all admins – their sharing management setting (not role select) should be disabled

So, why show these two modules are managed by anadmin when they are managed by all admins?

Screenshots

What Admin 1 sees  What Admin 2 sees 

[asvinb]

@wpdarren Kindly find my comments pertaining to the UI/UX points you raised (@hussain-t is looking at the other points):

• 2. This is as per designs where no external icons is present. However for the sake of consistency, I think we should add it. There is a follow up PR for that.
• 4. This has been implemented.
• 5. I think there is some inconsistency in the Figma designs itself. If you look at this screen, https://www.figma.com/file/i04x83uA9U8HHBANlhS7Db/Dashboard-Sharing?node-id=1214%3A18915, you will notice the font family used is Roboto, with a font weight of 500. In screens where Google Sans is being used, the font weight is 400 but it does not look good since the font must be bolded. Furthermore, to be consistent with table headers, I think we can keep the font family to Roboto and the font weight to 500.
• 6. Same as above.
• 7. Fixed.
• 8. So we have 2 states for the button. One is the hover state and the other one is the focus states which are different. So when the dialog is closed, the button gets the focus back, which has a background color which is different from the hover state. This is expected.
• 10. Maybe we need to create a followup issue for that since we don't have any designs on how it should look. I think to maximise the use of real estate screen, it's better to have the user roles to be 100% wide and then the dropdown goes below the roles.
• 11. See above.
• 12. The tooltips is managed outside of the modal, I don't think it's an issue to have part of it displayed outside of the modal.
• 13. The tooltips should work on small screens. If you touch the icon for a small amount of time, the tooltip will be displayed.

[hussain-t]

@wpdarren, I have fixed #3 and #4 in this follow-up PR. As for #1 and #9, this is a backend issue that only the modules available in the _googlesitekitDashboardSharingData.settings are changeable. @jimmymadon should be able to provide more context on this as he worked on the backed part of it.

[tofumatt]

As for #1 and #9, this is a backend issue that only the modules available in the _googlesitekitDashboardSharingData.settings are changeable. @jimmymadon should be able to provide more context on this as he worked on the backed part of it.

In that case, we should not be showing UI for modules we can't change. Do you think those modules should be editable? I'd think so, but maybe I'm not following.

Either they should be editable (preferred if they can be shared)… or if they totally can't be shared we shouldn't show them for now.

[hussain-t]

@tofumatt, as per the AC, the shared ownership modules should have All Admins and be disabled:

Rows for https://github.com/google/site-kit-wp/issues/4791 are special in that these are manageable by all admins – their sharing management setting (not role select) should be disabled

@wpdarren understood this point later and updated his post:

This might be linked with issue 3 above. While logged into an admin account, for PSI and Idea Hub module they are managed by another admin user anadmin. When I login as anadmin I am unable to change Who can manage view access as they are inactive dropdowns. See screenshots below Is this expected?

Update: I just spotted this in the AC: Rows for https://github.com/google/site-kit-wp/issues/4791 are special in that these are manageable by all admins – their sharing management setting (not role select) should be disabled

So, why show these two modules are managed by anadmin when they are managed by all admins?

So, So, why show these two modules are managed by anadmin when they are managed by all admins? is a valid issue and I have fixed it in my PR.

As for #1 and #9 that the roles aren't set after clicking Apply, as I mentioned in this comment that this is a backend issue. The module has to be registered in the database for changing the sharing settings. @jimmymadon, could you shed more light on this as you worked on the backend. Thanks!

[wpdarren]

@hussain-t @tofumatt re.

As for #1 and #9 that the roles aren't set after clicking Apply, as I mentioned in this https://github.com/google/site-kit-wp/issues/4822#issuecomment-1147640654 that this is a backend issue. The module has to be registered in the database for changing the sharing settings. @jimmymadon, could you shed more light on this as you worked on the backend. Thanks!

I am conscious that these are E2E blocking issues, and IMO, this ticket should not be moved to CR/MR and QA until they are fixed as they will need another PR.

Update: Sorry, I didn't realise that two issues were not linked with this ticket. Ignore me. 😄

[wpdarren]

QA Update: ⚠️

For issues 1 and 9, on my initial issues, it's my understanding that the cause of these issues are not part of this ticket. From chatting with @hussain-t it would appear that this issue is related to #4481 which is closed. @jimmymadon was investigating these issues, but wanted to highlight this here. These issues are going to cause a problem with completing our E2E testing as we are unable to assign roles to Search console, Analytics and AdSense modules. Plus we are unable to manage view access for these modules.

At the moment, I am unable to pass this because I am unable to add roles, or, manage view access for these modules.

~~ @hussain-t @tofumatt I still am unsure about issue 3 and 14 on my initial issues. The PSI and Idea Hub dropdown for Who can manage view access is disabled. I realise in the AC, it suggests that this is correct, but could we not have a scenario where an admin does not want to share these modules with all admins? Is this isn't a scenario, then should we hide these dropdown's to avoid confusion? If we are unsure and needs clarification then we can note this as a known issue to discuss at bug bash. ~~

Update: for issue 3 and 14, I am going to add it on to the known issue list as a UX question for the bug bash.

[wpdarren]

QA Update: ✅

Can confirm that issue 1 and 9 are now fixed 🥳

Verified:

• The design follows the relevant portions of the Figma document for dashboard sharing.
• Tested on small screen sizes and larger viewports
• The table is sorted by the modules the user has the most control over are at the top and least/no control over at the bottom
• The Single admin environments have columns for Product, Who can view
• Multi- admin environments have columns for Product, Who can view, Who can manage view access
• Checked that the admin and non admin users could see the appropriate data and widgets to check.

Managing user to manage view access

• For multi-admin environments, the select for a module's sharing management is replaced by additional information about the module's owner where admin-2 is the user's login/username: Managed by admin-2 with a tooltip.
• Submitting only happens if settings have changed
• A spinner displays while the submit action is in progress and all setting fields should be disabled
• When changes to user roles are made, an informative dialog is shown in the footer of the settings to inform the user of the the implications of the change.
• A similar but different message is shown if changing a module's settings management to "All admins" which takes precedence over the message about a change in selected roles.
• URLs for any Learn More links use https://sitekit.withgoogle.com/documentation/using-site-kit/dashboard-sharing/

Screenshots