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

Split SetupUsingProxy into sub-components for view-only and full-splash variants #4809

Closed
aaemnnosttv opened this issue Feb 8, 2022 · 4 comments
Labels
P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Feb 8, 2022

Feature Description

Until now, the splash screen has been where we welcome new users before sending them along to sign-in with Google which has been a required prerequisite for using Site Kit for all users. Starting with dashboard sharing, this will no longer be required for all users. When one or more modules are shared with a non-admin user's role, that user will see a new version of the splash screen as a stepping stone to view the shared dashboard. This will be a similar, but new version of the splash screen.

The current splash screen will be updated in #4811 to offer a view-only option to non-authenticated admins under similar circumstances, otherwise the splash will continue to look and work similar to today.

This issue is mostly about splitting the existing splash components into current and new non-admin shared-only variants and scaffolding new components and infrastructure.


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

Acceptance criteria

  • The SetupUsingProxy component should be split into two new components:
  • DashboardSplashApp should be updated to conditionally render one or the other in place of SetupUsingProxy (i.e. both still require usingProxy) based on the user's ability to authenticate and the dashboardSharing feature flag
    • If the user cannot authenticate and the dashboardSharing feature is enabled, render SetupUsingProxyViewOnly
    • Otherwise render SetupUsingProxyWithSignIn
  • Storybook stories for SetupUsingProxy should be updated to use SetupUsingProxyWithSignIn
    • The title should be updated to Setup / Using Proxy With Sign-in
  • Storybook stories should be scaffolded for SetupUsingProxyViewOnly
    • The title should be Setup / Using Proxy View-Only

Implementation Brief

  • Rename assets/js/components/setup/SetupUsingProxy.js to assets/js/components/setup/SetupUsingProxyWithSignIn.js, renaming the component as well.
  • Create assets/js/components/setup/SetupUsingProxyViewOnly.js which exports the SetupUsingProxyViewOnly functional component.
    • It should return the following untranslated string: TODO: UI to view only splash page..
  • Using assets/js/components/dashboard-splash/DashboardSplashApp.js,
    • Query the user data store via the hasCapability selector, passing PERMISSION_AUTHENTICATE to check if the user has the capability to authenticate with Site Kit.
    • Check if the dashboardSharing is enabled via the useFeature hook, passing dashboardSharing as parameter.
    • If the user cannot authenticate and the dashboardSharing feature flag is enabled and usingProxy is true, render the SetupUsingProxyViewOnly component. Otherwise, render the SetupUsingProxyWithSignIn component.
  • Rename assets/js/components/setup/SetupUsingProxy.stories.js to assets/js/components/setup/SetupUsingProxyWithSignIn.stories.js, updating the stories to use SetupUsingProxyWithSignIn as well.
  • Create assets/js/components/setup/SetupUsingProxyViewOnly.stories.js which will contain a single story for now.
  • Refer to the AC for the story titles.

Test Coverage

  • No new tests to be added.

QA Brief

  • Check the storybook Setup / Using Proxy View-Only / Start story. It should display the placeholder for the new setup component.

Changelog entry

  • N/A.
@aaemnnosttv aaemnnosttv added P0 High priority Type: Enhancement Improvement of an existing feature labels Feb 8, 2022
@aaemnnosttv aaemnnosttv self-assigned this Feb 8, 2022
@felixarntz
Copy link
Member

@aaemnnosttv ACs sgtm, just one thought: Do we think that the splash screen with sign in for when the view-only dashboard doesn't exist yet behaves closely enough to when the view-only dashboard does exist to implement it in the same component? I'm just raising this since we technically have 3 different UI permutations, but only 2 components. I'm not saying we should have 3 components, just wanted to raise this in case.

One more detail: That may be obvious to who reads these ACs, but clarifying that the old SetupUsingProxy should be removed may be helpful.

@aaemnnosttv
Copy link
Collaborator Author

Do we think that the splash screen with sign in for when the view-only dashboard doesn't exist yet behaves closely enough to when the view-only dashboard does exist to implement it in the same component?

@felixarntz my initial thought was that the difference between those two probably wouldn't justify separate components for each, although I think that's something that we could consider in the definition of #4811.

One more detail: That may be obvious to who reads these ACs, but clarifying that the old SetupUsingProxy should be removed may be helpful.

Sounds good, I'll update this to be more explicit 👍

@aaemnnosttv aaemnnosttv removed their assignment Mar 24, 2022
@asvinb asvinb assigned asvinb and unassigned asvinb Apr 4, 2022
@eugene-manuilov eugene-manuilov self-assigned this Apr 7, 2022
@eugene-manuilov
Copy link
Collaborator

IB ✔️

@wpdarren
Copy link
Collaborator

wpdarren commented Apr 19, 2022

QA Update: ✅

Verified:

Additional QA check using the React developer tool based on the ACs

  • SetupUsingProxyWithSignIn component was found on the splash page.
  • SetupUsingProxyViewOnly component wasn't found but is related to a ticket not yet executed.
  • SetupUsingProxy component is no longer found as expected.
  • DashboardSplashApp component was found when dashboardSharing is enabled.

image

@wpdarren wpdarren removed their assignment Apr 19, 2022
@wpdarren wpdarren reopened this Aug 5, 2022
@wpdarren wpdarren closed this as completed Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants