Browse Source

ref(ui): Various CSS cleanups in Issue Group Header (#39009)

Evan Purkhiser 2 years ago
parent
commit
0168e97858

+ 3 - 1
fixtures/page_objects/issue_details.py

@@ -68,7 +68,9 @@ class IssueDetailsPage(BasePage):
         self.browser.wait_until('[data-test-id="unbookmark"]')
 
     def assign_to(self, user):
-        assignee = self.browser.find_element(by=By.CSS_SELECTOR, value=".assigned-to")
+        assignee = self.browser.find_element(
+            by=By.CSS_SELECTOR, value='[data-test-id="assigned-to"]'
+        )
 
         # Open the assignee picker
         assignee.find_element(by=By.CSS_SELECTOR, value='[role="button"]').click()

+ 1 - 7
static/app/components/seenByList.tsx

@@ -1,6 +1,5 @@
 import {Fragment} from 'react';
 import styled from '@emotion/styled';
-import classNames from 'classnames';
 import moment from 'moment';
 
 import AvatarList from 'sentry/components/avatar/avatarList';
@@ -45,10 +44,7 @@ const SeenByList = ({
 
   // Note className="seen-by" is required for responsive design
   return (
-    <SeenByWrapper
-      iconPosition={iconPosition}
-      className={classNames('seen-by', className)}
-    >
+    <SeenByWrapper iconPosition={iconPosition} className={className}>
       <AvatarList
         users={displayUsers}
         avatarSize={avatarSize}
@@ -72,8 +68,6 @@ const SeenByList = ({
 
 const SeenByWrapper = styled('div')<{iconPosition: Props['iconPosition']}>`
   display: flex;
-  margin-top: 15px;
-  float: right;
   ${p => (p.iconPosition === 'left' ? 'flex-direction: row-reverse' : '')};
 `;
 

+ 1 - 5
static/app/views/eventsV2/eventDetails/linkedIssue.tsx

@@ -75,7 +75,7 @@ class LinkedIssue extends AsyncComponent<
                 }
               />
             </StyledLink>
-            <StyledSeenByList seenBy={group.seenBy} maxVisibleAvatars={5} />
+            <SeenByList seenBy={group.seenBy} maxVisibleAvatars={5} />
           </IssueCardHeader>
           <IssueCardBody>
             <GroupChart statsPeriod="30d" data={group} height={56} />
@@ -114,10 +114,6 @@ const IssueCardBody = styled('div')`
   padding-top: ${space(1)};
 `;
 
-const StyledSeenByList = styled(SeenByList)`
-  margin: 0;
-`;
-
 const StyledShortId = styled(ShortId)`
   font-size: ${p => p.theme.fontSizeMedium};
   color: ${p => p.theme.textColor};

+ 0 - 1
static/app/views/organizationGroupDetails/actions/index.tsx

@@ -487,7 +487,6 @@ const Wrapper = styled('div')`
   justify-content: flex-start;
   align-items: center;
   grid-auto-flow: column;
-  margin-top: ${space(2)};
   gap: ${space(0.5)};
   white-space: nowrap;
 `;

+ 99 - 117
static/app/views/organizationGroupDetails/header.tsx

@@ -31,7 +31,7 @@ import Tooltip from 'sentry/components/tooltip';
 import {IconChat} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
-import {Group, IssueCategory, Organization, Project} from 'sentry/types';
+import {Group, IssueCategory, Organization, Project, User} from 'sentry/types';
 import {Event} from 'sentry/types/event';
 import trackAdvancedAnalyticsEvent from 'sentry/utils/analytics/trackAdvancedAnalyticsEvent';
 import {getUtcDateString} from 'sentry/utils/dates';
@@ -56,12 +56,8 @@ type Props = WithRouterProps & {
   event?: Event;
 };
 
-type MemberList = NonNullable<
-  React.ComponentProps<typeof AssigneeSelector>['memberList']
->;
-
 type State = {
-  memberList?: MemberList;
+  memberList?: User[];
 };
 
 class GroupHeader extends Component<Props, State> {
@@ -173,26 +169,26 @@ class GroupHeader extends Component<Props, State> {
         >
           {t('Details')}
         </ListLink>
-        <StyledListLink
+        <ListLink
           to={`${baseUrl}activity/${location.search}`}
           isActive={() => currentTab === Tab.ACTIVITY}
           disabled={disabledTabs.includes(Tab.ACTIVITY)}
           onClick={() => this.tabClickAnalyticsEvent(Tab.ACTIVITY)}
         >
           {t('Activity')}
-          <Badge>
+          <IconBadge>
             {group.numComments}
             <IconChat size="xs" />
-          </Badge>
-        </StyledListLink>
-        <StyledListLink
+          </IconBadge>
+        </ListLink>
+        <ListLink
           to={`${baseUrl}feedback/${location.search}`}
           isActive={() => currentTab === Tab.USER_FEEDBACK}
           disabled={disabledTabs.includes(Tab.USER_FEEDBACK)}
           onClick={() => this.tabClickAnalyticsEvent(Tab.USER_FEEDBACK)}
         >
           {t('User Feedback')} <Badge text={group.userReportCount} />
-        </StyledListLink>
+        </ListLink>
         {hasEventAttachments && (
           <ListLink
             to={`${baseUrl}attachments/${location.search}`}
@@ -283,18 +279,18 @@ class GroupHeader extends Component<Props, State> {
         >
           {t('Details')}
         </ListLink>
-        <StyledListLink
+        <ListLink
           to={`${baseUrl}activity/${location.search}`}
           isActive={() => currentTab === Tab.ACTIVITY}
           disabled={disabledTabs.includes(Tab.ACTIVITY)}
           onClick={() => this.tabClickAnalyticsEvent(Tab.ACTIVITY)}
         >
           {t('Activity')}
-          <Badge>
+          <IconBadge>
             {group.numComments}
             <IconChat size="xs" />
-          </Badge>
-        </StyledListLink>
+          </IconBadge>
+        </ListLink>
         <ListLink
           to={`${baseUrl}tags/${location.search}`}
           isActive={() => currentTab === Tab.TAGS}
@@ -344,14 +340,14 @@ class GroupHeader extends Component<Props, State> {
 
     const shortIdBreadCrumb = group.shortId && (
       <GuideAnchor target="issue_number" position="bottom">
-        <IssueBreadcrumbWrapper>
-          <BreadcrumbProjectBadge
+        <ShortIdBreadrcumb>
+          <ProjectBadge
             project={project}
             avatarSize={16}
             hideName
             avatarProps={{hasTooltip: true, tooltip: project.slug}}
           />
-          <StyledTooltip
+          <Tooltip
             className="help-link"
             title={t(
               'This identifier is unique across your organization, and can be used to reference an issue in various places, like commit messages.'
@@ -359,21 +355,21 @@ class GroupHeader extends Component<Props, State> {
             position="bottom"
           >
             <StyledShortId shortId={group.shortId} />
-          </StyledTooltip>
+          </Tooltip>
           {group.issueCategory === IssueCategory.PERFORMANCE && (
             <FeatureBadge
               type="beta"
               title="Performance issues are available for early adopters and may change"
             />
           )}
-        </IssueBreadcrumbWrapper>
+        </ShortIdBreadrcumb>
       </GuideAnchor>
     );
 
     return (
       <Layout.Header>
         <div className={className}>
-          <StyledBreadcrumbs
+          <Breadcrumbs
             crumbs={[
               {
                 label: 'Issues',
@@ -384,69 +380,57 @@ class GroupHeader extends Component<Props, State> {
               },
             ]}
           />
-          <div className="row">
-            <div className="col-sm-7">
-              <TitleWrapper>
+          <HeaderRow>
+            <TitleWrapper>
+              <TitleHeading>
                 <h3>
                   <StyledEventOrGroupTitle hasGuideAnchor data={group} />
                 </h3>
-                {group.inbox && (
-                  <InboxReasonWrapper>
-                    <InboxReason inbox={group.inbox} fontSize="md" />
-                  </InboxReasonWrapper>
-                )}
-              </TitleWrapper>
+                {group.inbox && <InboxReason inbox={group.inbox} fontSize="md" />}
+              </TitleHeading>
               <StyledTagAndMessageWrapper>
                 {group.level && <ErrorLevel level={group.level} size="11px" />}
                 {group.isUnhandled && <UnhandledInboxTag />}
                 <EventMessage
                   message={message}
                   annotations={
-                    <Fragment>
-                      {group.logger && (
-                        <EventAnnotationWithSpace>
-                          <Link
-                            to={{
-                              pathname: `/organizations/${organization.slug}/issues/`,
-                              query: {query: 'logger:' + group.logger},
-                            }}
-                          >
-                            {group.logger}
-                          </Link>
-                        </EventAnnotationWithSpace>
-                      )}
-                    </Fragment>
+                    group.logger && (
+                      <EventAnnotation>
+                        <Link
+                          to={{
+                            pathname: `/organizations/${organization.slug}/issues/`,
+                            query: {query: 'logger:' + group.logger},
+                          }}
+                        >
+                          {group.logger}
+                        </Link>
+                      </EventAnnotation>
+                    )
                   }
                 />
               </StyledTagAndMessageWrapper>
-            </div>
-
+            </TitleWrapper>
             <StatsWrapper>
-              <div className="count align-right m-l-1">
+              <div className="count">
                 <h6 className="nav-header">{t('Events')}</h6>
-                {disableActions ? (
+                <Link disabled={disableActions} to={eventRouteToObject}>
                   <Count className="count" value={group.count} />
-                ) : (
-                  <Link to={eventRouteToObject}>
-                    <Count className="count" value={group.count} />
-                  </Link>
-                )}
+                </Link>
               </div>
-              <div className="count align-right m-l-1">
+              <div className="count">
                 <h6 className="nav-header">{t('Users')}</h6>
                 {userCount !== 0 ? (
-                  disableActions ? (
+                  <Link
+                    disabled={disableActions}
+                    to={`${baseUrl}tags/user/${location.search}`}
+                  >
                     <Count className="count" value={userCount} />
-                  ) : (
-                    <Link to={`${baseUrl}tags/user/${location.search}`}>
-                      <Count className="count" value={userCount} />
-                    </Link>
-                  )
+                  </Link>
                 ) : (
                   <span>0</span>
                 )}
               </div>
-              <div className="assigned-to m-l-1">
+              <div data-test-id="assigned-to">
                 <h6 className="nav-header">{t('Assignee')}</h6>
                 <AssigneeSelector
                   id={group.id}
@@ -456,18 +440,20 @@ class GroupHeader extends Component<Props, State> {
                 />
               </div>
             </StatsWrapper>
-          </div>
-          <SeenByList
-            seenBy={group.seenBy}
-            iconTooltip={t('People who have viewed this issue')}
-          />
-          <GroupActions
-            group={group}
-            project={project}
-            disabled={disableActions}
-            event={event}
-            query={location.query}
-          />
+          </HeaderRow>
+          <HeaderRow>
+            <GroupActions
+              group={group}
+              project={project}
+              disabled={disableActions}
+              event={event}
+              query={location.query}
+            />
+            <StyledSeenByList
+              seenBy={group.seenBy}
+              iconTooltip={t('People who have viewed this issue')}
+            />
+          </HeaderRow>
           <NavTabs>
             {group.issueCategory === IssueCategory.PERFORMANCE
               ? this.getPerformanceIssueTabs()
@@ -481,73 +467,69 @@ class GroupHeader extends Component<Props, State> {
 
 export default withApi(withRouter(withOrganization(GroupHeader)));
 
-const TitleWrapper = styled('div')`
+const ShortIdBreadrcumb = styled('div')`
   display: flex;
-  line-height: 24px;
+  gap: ${space(1)};
+  align-items: center;
 `;
 
-const StyledEventOrGroupTitle = styled(EventOrGroupTitle)`
-  font-size: inherit;
+const StyledShortId = styled(ShortId)`
+  font-family: ${p => p.theme.text.family};
+  font-size: ${p => p.theme.fontSizeMedium};
+`;
+
+const HeaderRow = styled('div')`
+  display: flex;
+  gap: ${space(2)};
+  justify-content: space-between;
+  margin-top: ${space(2)};
+
+  @media (max-width: ${p => p.theme.breakpoints.small}) {
+    flex-direction: column;
+  }
 `;
 
-const StyledBreadcrumbs = styled(Breadcrumbs)`
-  margin-bottom: ${space(2)};
+const TitleWrapper = styled('div')`
+  @media (min-width: ${p => p.theme.breakpoints.small}) {
+    max-width: 65%;
+  }
 `;
 
-const IssueBreadcrumbWrapper = styled('div')`
+const TitleHeading = styled('div')`
   display: flex;
-  align-items: center;
+  line-height: 2;
+  gap: ${space(1)};
 `;
 
-const StyledTooltip = styled(Tooltip)`
-  display: flex;
+const StyledSeenByList = styled(SeenByList)`
+  @media (max-width: ${p => p.theme.breakpoints.small}) {
+    display: none;
+  }
 `;
 
-const StyledShortId = styled(ShortId)`
-  font-family: ${p => p.theme.text.family};
-  font-size: ${p => p.theme.fontSizeMedium};
+const StyledEventOrGroupTitle = styled(EventOrGroupTitle)`
+  font-size: inherit;
 `;
 
 const StatsWrapper = styled('div')`
   display: grid;
-  justify-content: flex-end;
   grid-template-columns: repeat(3, min-content);
-  gap: ${space(3)};
-  margin-right: 15px;
-`;
+  gap: calc(${space(3)} + ${space(3)});
 
-const InboxReasonWrapper = styled('div')`
-  margin-left: ${space(1)};
+  @media (min-width: ${p => p.theme.breakpoints.small}) {
+    justify-content: flex-end;
+  }
 `;
 
 const StyledTagAndMessageWrapper = styled(TagAndMessageWrapper)`
-  display: grid;
-  grid-auto-flow: column;
+  display: flex;
   gap: ${space(1)};
   justify-content: flex-start;
   line-height: 1.2;
-
-  @media (max-width: ${p => p.theme.breakpoints.small}) {
-    margin-bottom: ${space(2)};
-  }
 `;
 
-const StyledListLink = styled(ListLink)`
-  svg {
-    margin-left: ${space(0.5)};
-    margin-bottom: ${space(0.25)};
-    vertical-align: middle;
-  }
-`;
-
-const StyledProjectBadge = styled(ProjectBadge)`
-  flex-shrink: 0;
-`;
-
-const BreadcrumbProjectBadge = styled(StyledProjectBadge)`
-  margin-right: ${space(0.75)};
-`;
-
-const EventAnnotationWithSpace = styled(EventAnnotation)`
-  margin-left: ${space(1)};
+const IconBadge = styled(Badge)`
+  display: flex;
+  align-items: center;
+  gap: ${space(0.5)};
 `;

+ 0 - 4
static/less/group-detail.less

@@ -1326,10 +1326,6 @@ ul.crumbs {
     .is-assigned {
       padding-top: 0;
     }
-
-    .seen-by {
-      display: none;
-    }
   }
 
   // Event detail columns