Browse Source

bootstrap: wrap components inside a theme and style provider (#85081)

Some components are directly importing the theme object, which means
that they will only ever use light color scheme and will not be reactive
to the theme preference set by the user (the colors in this case might
look off or broken).

I have noticed that some components like the setupWizard and
superUserStaffAccessForm already define these internally, meaning that
we should be safe to use it everywhere and not have to rely on direct
theme usage.
Jonas 4 weeks ago
parent
commit
567d9dfcb4

+ 176 - 0
static/app/bootstrap/processInitQueue.spec.tsx

@@ -1,6 +1,182 @@
+import {OrganizationFixture} from 'sentry-fixture/organization';
+import {ProjectFixture} from 'sentry-fixture/project';
+import {TeamFixture} from 'sentry-fixture/team';
+
+import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
+
 import {processInitQueue} from 'sentry/bootstrap/processInitQueue';
+import AlertStore from 'sentry/stores/alertStore';
+import IndicatorStore from 'sentry/stores/indicatorStore';
+import {SentryInitRenderReactComponent} from 'sentry/types/system';
 
 describe('processInitQueue', function () {
+  describe('renderReact', function () {
+    it('renders password strength input', async () => {
+      window.__onSentryInit = [
+        {
+          name: 'passwordStrength',
+          input: '#password',
+          element: '#password-strength',
+        },
+      ];
+
+      render(
+        <div>
+          <input id="password" placeholder="password" />
+          <div id="password-strength" />
+        </div>
+      );
+
+      processInitQueue();
+
+      // Assert that password strength renders and reacts to user input
+      await userEvent.type(screen.getByPlaceholderText('password'), '!');
+      expect(await screen.findByText('Very Weak')).toBeInTheDocument();
+
+      // Type the rest of the password
+      await userEvent.type(
+        screen.getByPlaceholderText('password'),
+        '!!!!!supersecretpassword!!!!!!'
+      );
+      expect(await screen.findByText('Very Strong')).toBeInTheDocument();
+    });
+
+    it('renders indicators', async () => {
+      window.__onSentryInit = [
+        {
+          component: SentryInitRenderReactComponent.INDICATORS,
+          container: '#indicator-container',
+          name: 'renderReact',
+        },
+      ];
+
+      IndicatorStore.add('Indicator Alert', 'success');
+
+      render(<div id="indicator-container" />);
+      processInitQueue();
+      expect(await screen.findByText('Indicator Alert')).toBeInTheDocument();
+    });
+    it('renders system alerts', async () => {
+      window.__onSentryInit = [
+        {
+          component: SentryInitRenderReactComponent.SYSTEM_ALERTS,
+          container: '#system-alerts-container',
+          name: 'renderReact',
+        },
+      ];
+
+      AlertStore.addAlert({
+        message: 'System Alert',
+        type: 'success',
+      });
+
+      render(<div id="system-alerts-container" />);
+      processInitQueue();
+      expect(await screen.findByText('System Alert')).toBeInTheDocument();
+    });
+    it('renders setup wizard', async () => {
+      window.__onSentryInit = [
+        {
+          component: SentryInitRenderReactComponent.SETUP_WIZARD,
+          container: '#setup-wizard-container',
+          name: 'renderReact',
+          props: {
+            enableProjectSelection: true,
+            hash: '1',
+          },
+        },
+      ];
+
+      MockApiClient.addMockResponse({
+        url: '/organizations/',
+        body: [
+          OrganizationFixture({
+            id: '1',
+            slug: 'organization-1',
+            name: 'Organization 1',
+            access: [],
+            features: [],
+          }),
+        ],
+      });
+
+      MockApiClient.addMockResponse({
+        url: '/organizations/organization-1/',
+        body: OrganizationFixture({
+          id: '1',
+          slug: 'organization-1',
+          name: 'Organization 1',
+          features: [],
+          access: [],
+        }),
+      });
+
+      MockApiClient.addMockResponse({
+        url: '/organizations/organization-1/projects/',
+        body: [
+          ProjectFixture({
+            id: '1',
+            slug: 'project-1',
+            name: 'Project 1',
+          }),
+        ],
+      });
+
+      MockApiClient.addMockResponse({
+        url: '/organizations/organization-1/teams/',
+        body: [TeamFixture({id: '1', slug: 'team-1', name: 'Team 1'})],
+      });
+
+      render(<div id="setup-wizard-container" />);
+      processInitQueue();
+
+      expect(await screen.findByText('Select your Sentry project')).toBeInTheDocument();
+    });
+    it('renders u2f sign', async () => {
+      window.__onSentryInit = [
+        {
+          component: SentryInitRenderReactComponent.U2F_SIGN,
+          container: '#u2f-sign-container',
+          name: 'renderReact',
+          props: {
+            displayMode: 'signin',
+          },
+        },
+      ];
+
+      render(<div id="u2f-sign-container" />);
+      processInitQueue();
+
+      // U2F is not supported in the test environment
+      expect(
+        await screen.findByText(/Unfortunately your browser does not support U2F/)
+      ).toBeInTheDocument();
+    });
+
+    it('renders superuser staff access form', async () => {
+      window.__onSentryInit = [
+        {
+          component: SentryInitRenderReactComponent.SU_STAFF_ACCESS_FORM,
+          container: '#su-staff-access-form-container',
+          name: 'renderReact',
+        },
+      ];
+
+      const authenticatorsResponse = MockApiClient.addMockResponse({
+        url: '/authenticators/',
+        body: [],
+      });
+
+      render(<div id="su-staff-access-form-container" />);
+      processInitQueue();
+
+      await waitFor(() => {
+        expect(authenticatorsResponse).toHaveBeenCalled();
+      });
+      expect(await screen.findByText('COPS/CSM')).toBeInTheDocument();
+    });
+  });
+
   it('processes queued up items', function () {
     const mock = jest.fn();
     const init = {

+ 29 - 6
static/app/bootstrap/processInitQueue.tsx

@@ -2,6 +2,7 @@ import {createRoot} from 'react-dom/client';
 import throttle from 'lodash/throttle';
 
 import {exportedGlobals} from 'sentry/bootstrap/exportGlobals';
+import {ThemeAndStyleProvider} from 'sentry/components/themeAndStyleProvider';
 import type {OnSentryInitConfiguration} from 'sentry/types/system';
 import {SentryInitRenderReactComponent} from 'sentry/types/system';
 
@@ -31,12 +32,12 @@ async function processItem(initConfig: OnSentryInitConfiguration) {
    * password strength estimation library. Load it on demand.
    */
   if (initConfig.name === 'passwordStrength') {
-    const {input, element} = initConfig;
-    if (!input || !element) {
+    if (!initConfig.input || !initConfig.element) {
       return;
     }
-    const inputElem = document.querySelector(input);
-    const rootEl = document.querySelector(element);
+    const inputElem = document.querySelector(initConfig.input);
+    const rootEl = document.querySelector(initConfig.element);
+
     if (!inputElem || !rootEl) {
       return;
     }
@@ -49,7 +50,16 @@ async function processItem(initConfig: OnSentryInitConfiguration) {
     inputElem.addEventListener(
       'input',
       throttle(e => {
-        root.render(<PasswordStrength value={e.target.value} />);
+        root.render(
+          /**
+           * The screens and components rendering here will always render in light mode.
+           * This is because config is not available at this point (user might not be logged in yet),
+           * and so we dont know which theme to pick.
+           */
+          <ThemeAndStyleProvider>
+            <PasswordStrength value={e.target.value} />
+          </ThemeAndStyleProvider>
+        );
       })
     );
 
@@ -68,7 +78,20 @@ async function processItem(initConfig: OnSentryInitConfiguration) {
 
     renderOnDomReady(() =>
       // TODO(ts): Unsure how to type this, complains about u2fsign's required props
-      renderDom(Component as any, initConfig.container, initConfig.props)
+      renderDom(
+        (props: any) => (
+          /**
+           * The screens and components rendering here will always render in light mode.
+           * This is because config is not available at this point (user might not be logged in yet),
+           * and so we dont know which theme to pick.
+           */
+          <ThemeAndStyleProvider>
+            <Component {...props} />
+          </ThemeAndStyleProvider>
+        ),
+        initConfig.container,
+        initConfig.props
+      )
     );
   }
 

+ 4 - 3
static/app/bootstrap/renderDom.tsx

@@ -1,9 +1,10 @@
+import type React from 'react';
 import {createRoot} from 'react-dom/client';
 
-export function renderDom(
-  Component: React.ComponentType,
+export function renderDom<T extends React.ComponentType<any>>(
+  Component: T,
   container: string,
-  props: Record<string, any> = {}
+  props: React.ComponentProps<T>
 ) {
   const rootEl = document.querySelector(container);
 

+ 1 - 1
static/app/bootstrap/renderMain.tsx

@@ -5,7 +5,7 @@ import {renderDom} from './renderDom';
 
 export function renderMain() {
   try {
-    renderDom(Main, `#${ROOT_ELEMENT}`);
+    renderDom(Main, `#${ROOT_ELEMENT}`, {});
   } catch (err) {
     if (err.message === 'URI malformed') {
       // eslint-disable-next-line no-console

+ 4 - 5
static/app/components/passwordStrength.tsx

@@ -47,11 +47,10 @@ export function PasswordStrength({
     return null;
   }
 
-  const {score} = result;
-  const percent = Math.round(((score + 1) / MAX_SCORE) * 100);
+  const percent = Math.round(((result.score + 1) / MAX_SCORE) * 100);
 
   const styles = css`
-    background: ${colors[score]};
+    background: ${colors[result.score]};
     width: ${percent}%;
   `;
 
@@ -59,7 +58,7 @@ export function PasswordStrength({
     <Fragment>
       <StrengthProgress
         role="progressbar"
-        aria-valuenow={score}
+        aria-valuenow={result.score}
         aria-valuemin={0}
         aria-valuemax={100}
       >
@@ -67,7 +66,7 @@ export function PasswordStrength({
       </StrengthProgress>
       <StrengthLabel>
         {tct('Strength: [textScore]', {
-          textScore: <ScoreText>{labels[score]}</ScoreText>,
+          textScore: <ScoreText>{labels[result.score]}</ScoreText>,
         })}
       </StrengthLabel>
     </Fragment>

+ 3 - 4
static/app/components/superuserStaffAccessForm.tsx

@@ -1,4 +1,4 @@
-import React, {Component, useState} from 'react';
+import React, {Component, Fragment, useState} from 'react';
 import {createBrowserRouter, RouterProvider} from 'react-router-dom';
 import styled from '@emotion/styled';
 
@@ -9,7 +9,6 @@ import {Button} from 'sentry/components/button';
 import Form from 'sentry/components/forms/form';
 import Hook from 'sentry/components/hook';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
-import {ThemeAndStyleProvider} from 'sentry/components/themeAndStyleProvider';
 import U2fContainer from 'sentry/components/u2f/u2fContainer';
 import {ErrorCodes} from 'sentry/constants/superuserAccessErrors';
 import {t} from 'sentry/locale';
@@ -196,7 +195,7 @@ class SuperuserStaffAccessFormContent extends Component<Props, State> {
     }
 
     return (
-      <ThemeAndStyleProvider>
+      <Fragment>
         {this.props.hasStaff ? (
           isLoading ? (
             <LoadingIndicator />
@@ -243,7 +242,7 @@ class SuperuserStaffAccessFormContent extends Component<Props, State> {
             )}
           </Form>
         )}
-      </ThemeAndStyleProvider>
+      </Fragment>
     );
   }
 }

+ 3 - 6
static/app/views/setupWizard/index.tsx

@@ -3,7 +3,6 @@ import {createBrowserRouter, RouterProvider} from 'react-router-dom';
 
 import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
-import {ThemeAndStyleProvider} from 'sentry/components/themeAndStyleProvider';
 import {t} from 'sentry/locale';
 import {
   DEFAULT_QUERY_CLIENT_CONFIG,
@@ -38,11 +37,9 @@ function SetupWizard({hash, enableProjectSelection = false}: Props) {
   );
 
   return (
-    <ThemeAndStyleProvider>
-      <QueryClientProvider client={queryClient}>
-        <RouterProvider router={router} />
-      </QueryClientProvider>
-    </ThemeAndStyleProvider>
+    <QueryClientProvider client={queryClient}>
+      <RouterProvider router={router} />
+    </QueryClientProvider>
   );
 }