Browse Source

fix(sidebar): Fix timing hazard when navigating away with sidebar open (#34031)

* fix(sidebar): Fix timing issue when navigating away with sidebar open

* fix testse

* use some ticks
Zhixing Zhang 2 years ago
parent
commit
15d9308618

+ 1 - 1
static/app/components/sidebar/sidebarPanel.tsx

@@ -90,7 +90,7 @@ function SidebarPanel({
     window.setTimeout(() => document.addEventListener('click', panelCloseHandler));
 
     return function cleanup() {
-      document.removeEventListener('click', panelCloseHandler);
+      window.setTimeout(() => document.removeEventListener('click', panelCloseHandler));
     };
   }, [panelCloseHandler]);
 

+ 21 - 9
tests/js/spec/components/sidebar/index.spec.jsx

@@ -36,8 +36,9 @@ describe('Sidebar', function () {
     });
   });
 
-  it('renders', function () {
-    renderSidebar();
+  it('renders', async function () {
+    const {container} = renderSidebar();
+    await waitFor(() => container);
     expect(screen.getByTestId('sidebar-dropdown')).toBeInTheDocument();
   });
 
@@ -73,8 +74,9 @@ describe('Sidebar', function () {
     window.location.assign.mockRestore();
   });
 
-  it('can toggle help menu', function () {
+  it('can toggle help menu', async function () {
     const {container} = renderSidebar();
+    await waitFor(() => container);
 
     userEvent.click(screen.getByText('Help'));
 
@@ -83,8 +85,9 @@ describe('Sidebar', function () {
   });
 
   describe('SidebarDropdown', function () {
-    it('can open Sidebar org/name dropdown menu', function () {
+    it('can open Sidebar org/name dropdown menu', async function () {
       const {container} = renderSidebar();
+      await waitFor(() => container);
 
       userEvent.click(screen.getByTestId('sidebar-dropdown'));
 
@@ -92,21 +95,22 @@ describe('Sidebar', function () {
       expect(orgSettingsLink).toBeInTheDocument();
       expect(container).toSnapshot();
     });
-
-    it('has link to Members settings with `member:write`', function () {
-      renderSidebar({
+    it('has link to Members settings with `member:write`', async function () {
+      const {container} = renderSidebar({
         organization: TestStubs.Organization({access: ['member:read']}),
       });
+      await waitFor(() => container);
 
       userEvent.click(screen.getByTestId('sidebar-dropdown'));
 
       expect(screen.getByText('Members')).toBeInTheDocument();
     });
 
-    it('can open "Switch Organization" sub-menu', function () {
+    it('can open "Switch Organization" sub-menu', async function () {
       act(() => void ConfigStore.set('features', new Set(['organizations:create'])));
 
       const {container} = renderSidebar();
+      await waitFor(() => container);
 
       userEvent.click(screen.getByTestId('sidebar-dropdown'));
 
@@ -133,6 +137,7 @@ describe('Sidebar', function () {
       rerender(getElement({location: {...router.location, pathname: 'new-path-name'}}));
 
       expect(screen.queryByText("What's new in Sentry")).not.toBeInTheDocument();
+      await tick();
     });
 
     it('can have onboarding feature', async function () {
@@ -147,6 +152,10 @@ describe('Sidebar', function () {
 
       expect(await screen.findByRole('dialog')).toBeInTheDocument();
       expect(screen.getByText('Capture your first error')).toBeInTheDocument();
+
+      userEvent.click(quickStart);
+      expect(screen.queryByText('Capture your first error')).not.toBeInTheDocument();
+      await tick();
     });
 
     it('displays empty panel when there are no Broadcasts', async function () {
@@ -167,6 +176,7 @@ describe('Sidebar', function () {
       // Close the sidebar
       userEvent.click(screen.getByText("What's new"));
       expect(screen.queryByText("What's new in Sentry")).not.toBeInTheDocument();
+      await tick();
     });
 
     it('can display Broadcasts panel and mark as seen', async function () {
@@ -199,6 +209,7 @@ describe('Sidebar', function () {
       // Close the sidebar
       userEvent.click(screen.getByText("What's new"));
       expect(screen.queryByText("What's new in Sentry")).not.toBeInTheDocument();
+      await tick();
     });
 
     it('can unmount Sidebar (and Broadcasts) and kills Broadcast timers', async function () {
@@ -235,7 +246,8 @@ describe('Sidebar', function () {
   });
 
   it('can toggle collapsed state', async function () {
-    renderSidebar();
+    const container = renderSidebar();
+    await waitFor(() => container);
 
     expect(screen.getByText(user.name)).toBeInTheDocument();
     expect(screen.getByText(organization.name)).toBeInTheDocument();