Browse Source

feat(discover-quick-context): Release Design Changes (#41617)

1. Removed first event and status.
2. Changed placements.
3. Fixed tests.

Co-authored-by: Abdullah Khan <abdullahkhan@P40P69L0VQ-Abdullah-Khan.local>
Abdkhan14 2 years ago
parent
commit
7fb572fc89

+ 8 - 54
static/app/views/eventsV2/table/quickContext.spec.tsx

@@ -1,13 +1,7 @@
 import {browserHistory} from 'react-router';
 import {QueryClient, QueryClientProvider} from '@tanstack/react-query';
 
-import {
-  render,
-  screen,
-  userEvent,
-  waitFor,
-  within,
-} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
 
 import ConfigStore from 'sentry/stores/configStore';
 import {Commit, Repository, User} from 'sentry/types';
@@ -376,7 +370,7 @@ describe('Quick Context', function () {
       expect(screen.getByTestId('version-hover-header-copy-icon')).toBeInTheDocument();
     });
 
-    it('Renders Release details for active release', async () => {
+    it('Renders Release details for release', async () => {
       MockApiClient.addMockResponse({
         url: '/organizations/org-slug/releases/backend@22.10.0+aaf33944f93dc8fa4234ca046a8d88fb1dccfb76/',
         body: mockedReleaseWithHealth,
@@ -386,38 +380,12 @@ describe('Quick Context', function () {
 
       userEvent.hover(screen.getByText('Text from Child'));
 
-      expect(await screen.findByText(/Release Details/i)).toBeInTheDocument();
-
-      const definitions = screen.getAllByRole('definition');
-      const terms = screen.getAllByRole('term');
-
-      expect(within(terms[0]).getByText(/Status/i)).toBeInTheDocument();
-      expect(within(definitions[0]).getByText(/Active/i)).toBeInTheDocument();
-      expect(within(terms[1]).getByText(/Created/i)).toBeInTheDocument();
-      expect(within(definitions[1]).getByText(/7 years ago/i)).toBeInTheDocument();
-      expect(within(terms[2]).getByText(/First Event/i)).toBeInTheDocument();
-      expect(within(definitions[2]).getByText(/7 years ago/i)).toBeInTheDocument();
-      expect(within(terms[3]).getByText(/Last Event/i)).toBeInTheDocument();
-      expect(within(definitions[3]).getByText(/6 years ago/i)).toBeInTheDocument();
-    });
-
-    it('Renders Release details for archived release', async () => {
-      MockApiClient.addMockResponse({
-        url: '/organizations/org-slug/releases/backend@22.10.0+aaf33944f93dc8fa4234ca046a8d88fb1dccfb76/',
-        body: {...mockedReleaseWithHealth, status: 'closed'},
-      });
-
-      renderQuickContextContent(defaultRow, ContextType.RELEASE);
-
-      userEvent.hover(screen.getByText('Text from Child'));
-
-      expect(await screen.findByText(/Release Details/i)).toBeInTheDocument();
-
-      expect(screen.getByText(/Status/i)).toBeInTheDocument();
-      expect(screen.getByText(/Archived/i)).toBeInTheDocument();
-      expect(screen.queryByText(/Created/i)).not.toBeInTheDocument();
-      expect(screen.queryByText(/First Event/i)).not.toBeInTheDocument();
-      expect(screen.queryByText(/Last Event/i)).not.toBeInTheDocument();
+      expect(await screen.findByText(/Created/i)).toBeInTheDocument();
+      expect(screen.getByText(/7 years ago/i)).toBeInTheDocument();
+      expect(screen.getByText(/Last Event/i)).toBeInTheDocument();
+      expect(screen.getByText(/6 years ago/i)).toBeInTheDocument();
+      expect(screen.getByText(/New Issues/i)).toBeInTheDocument();
+      expect(screen.getByText(/21/i)).toBeInTheDocument();
     });
 
     it('Renders Last Commit', async () => {
@@ -434,20 +402,6 @@ describe('Quick Context', function () {
       expect(screen.getByTestId('quick-context-commit-row')).toBeInTheDocument();
     });
 
-    it('Renders New Issues Count', async () => {
-      MockApiClient.addMockResponse({
-        url: '/organizations/org-slug/releases/backend@22.10.0+aaf33944f93dc8fa4234ca046a8d88fb1dccfb76/',
-        body: mockedReleaseWithHealth,
-      });
-
-      renderQuickContextContent(defaultRow, ContextType.RELEASE);
-
-      userEvent.hover(screen.getByText('Text from Child'));
-
-      expect(await screen.findByText(/New Issues/i)).toBeInTheDocument();
-      expect(screen.getByText(/21/i)).toBeInTheDocument();
-    });
-
     it('Renders Commit Count and Author when user is NOT in list of authors', async () => {
       MockApiClient.addMockResponse({
         url: '/organizations/org-slug/releases/backend@22.10.0+aaf33944f93dc8fa4234ca046a8d88fb1dccfb76/',

+ 38 - 53
static/app/views/eventsV2/table/quickContext.tsx

@@ -1,4 +1,4 @@
-import {Fragment, useEffect} from 'react';
+import {useEffect} from 'react';
 import {browserHistory} from 'react-router';
 import styled from '@emotion/styled';
 import {Location} from 'history';
@@ -15,7 +15,6 @@ import {
   StackTracePreviewContent,
 } from 'sentry/components/groupPreviewTooltip/stackTracePreview';
 import {Body, Hovercard} from 'sentry/components/hovercard';
-import {KeyValueTable, KeyValueTableRow} from 'sentry/components/keyValueTable';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import {Panel} from 'sentry/components/panels';
 import * as SidebarSection from 'sentry/components/sidebarSection';
@@ -285,38 +284,20 @@ function ReleaseContext(props: BaseContextProps) {
     );
   };
 
-  const renderReleaseDetails = () => {
-    const statusText = data?.status === 'open' ? t('Active') : t('Archived');
+  const renderReleaseAuthors = () => {
     return (
-      <ReleaseContextContainer data-test-id="quick-context-release-details-container">
-        <ContextHeader>
-          {t('Release Details')}
-          <FeatureBadge type="alpha" />
-        </ContextHeader>
-        <ContextBody>
-          <StyledKeyValueTable>
-            <KeyValueTableRow keyName={t('Status')} value={statusText} />
-            {data?.status === 'open' && (
-              <Fragment>
-                <KeyValueTableRow
-                  keyName={t('Created')}
-                  value={<TimeSince date={data.dateCreated} />}
-                />
-                <KeyValueTableRow
-                  keyName={t('First Event')}
-                  value={
-                    data.firstEvent ? <TimeSince date={data.firstEvent} /> : '\u2014'
-                  }
-                />
-                <KeyValueTableRow
-                  keyName={t('Last Event')}
-                  value={data.lastEvent ? <TimeSince date={data.lastEvent} /> : '\u2014'}
-                />
-              </Fragment>
+      data && (
+        <ReleaseContextContainer data-test-id="quick-context-release-details-container">
+          <ReleaseAuthorsTitle>{getCommitAuthorTitle()}</ReleaseAuthorsTitle>
+          <ReleaseAuthorsBody>
+            {data.commitCount === 0 ? (
+              <IconNot color="gray500" size="md" />
+            ) : (
+              <StyledAvatarList users={data.authors} maxVisibleAvatars={10} />
             )}
-          </StyledKeyValueTable>
-        </ContextBody>
-      </ReleaseContextContainer>
+          </ReleaseAuthorsBody>
+        </ReleaseContextContainer>
+      )
     );
   };
 
@@ -333,23 +314,25 @@ function ReleaseContext(props: BaseContextProps) {
       </ReleaseContextContainer>
     );
 
-  const renderIssueCountAndAuthors = () =>
+  const renderReleaseDetails = () =>
     data && (
       <ReleaseContextContainer data-test-id="quick-context-release-issues-and-authors-container">
         <ContextRow>
           <div>
-            <ContextHeader>{t('New Issues')}</ContextHeader>
-            <ReleaseStatusBody>{data.newGroups}</ReleaseStatusBody>
+            <ContextHeader>{t('Created')}</ContextHeader>
+            <ReleaseBody>
+              <TimeSince date={data.dateCreated} />
+            </ReleaseBody>
           </div>
           <div>
-            <ReleaseAuthorsTitle>{getCommitAuthorTitle()}</ReleaseAuthorsTitle>
-            <ReleaseAuthorsBody>
-              {data.commitCount === 0 ? (
-                <IconNot color="gray500" size="md" />
-              ) : (
-                <AvatarList users={data.authors} />
-              )}
-            </ReleaseAuthorsBody>
+            <ContextHeader>{t('Last Event')}</ContextHeader>
+            <ReleaseBody>
+              {data.lastEvent ? <TimeSince date={data.lastEvent} /> : '\u2014'}
+            </ReleaseBody>
+          </div>
+          <div>
+            <ContextHeader>{t('New Issues')}</ContextHeader>
+            <ContextBody>{data.newGroups}</ContextBody>
           </div>
         </ContextRow>
       </ReleaseContextContainer>
@@ -362,7 +345,7 @@ function ReleaseContext(props: BaseContextProps) {
   return (
     <Wrapper data-test-id="quick-context-hover-body">
       {renderReleaseDetails()}
-      {renderIssueCountAndAuthors()}
+      {renderReleaseAuthors()}
       {renderLastCommit()}
     </Wrapper>
   );
@@ -517,6 +500,7 @@ export function QuickContextHoverWrapper(props: ContextProps) {
     <HoverWrapper>
       <StyledHovercard
         showUnderline
+        displayTimeout={400}
         delay={HOVER_DELAY}
         header={getHoverHeader(dataRow, contextType)}
         body={getHoverBody(
@@ -596,6 +580,11 @@ const ContextBody = styled('div')`
   align-items: center;
 `;
 
+const ReleaseBody = styled(ContextBody)<{}>`
+  font-size: 13px;
+  color: ${p => p.theme.subText};
+`;
+
 const EventContextBody = styled(ContextBody)`
   font-size: ${p => p.theme.fontSizeExtraLarge};
   margin: 0;
@@ -611,7 +600,7 @@ const StatusText = styled('span')`
 const Wrapper = styled('div')`
   background: ${p => p.theme.background};
   border-radius: ${p => p.theme.borderRadius};
-  width: 300px;
+  width: 320px;
   padding: ${space(1.5)};
 `;
 
@@ -627,11 +616,6 @@ const NoContextWrapper = styled('div')`
   white-space: nowrap;
 `;
 
-const StyledKeyValueTable = styled(KeyValueTable)`
-  width: 100%;
-  margin: 0;
-`;
-
 const ReleaseContextContainer = styled(ContextContainer)`
   ${Panel} {
     margin: 0;
@@ -663,11 +647,12 @@ const ContextRow = styled('div')`
 `;
 
 const ReleaseAuthorsBody = styled(ContextBody)`
-  justify-content: right;
+  justify-content: left;
+  margin: 0;
 `;
 
-const ReleaseStatusBody = styled('h4')`
-  margin-bottom: 0;
+const StyledAvatarList = styled(AvatarList)`
+  margin: 0 ${space(0.75)};
 `;
 
 const StackTraceWrapper = styled('div')`