Browse Source

ref(ui): Simplify badges implementation (#68946)

- The UserBadge no longer has a bunch of custom styling and uses the
  existing styles

- The MemberBadge delegates to the User badge now

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Evan Purkhiser 11 months ago
parent
commit
4f24d94451

+ 1 - 1
static/app/components/idBadge/baseBadge.spec.tsx

@@ -2,7 +2,7 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
 
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
-import BaseBadge from 'sentry/components/idBadge/baseBadge';
+import {BaseBadge} from 'sentry/components/idBadge/baseBadge';
 
 describe('BadgeBadge', function () {
   it('has a display name', function () {

+ 12 - 14
static/app/components/idBadge/baseBadge.tsx

@@ -3,10 +3,9 @@ import styled from '@emotion/styled';
 
 import Avatar from 'sentry/components/avatar';
 import {space} from 'sentry/styles/space';
-import type {AvatarProject, Organization, Team} from 'sentry/types';
+import type {AvatarProject, AvatarUser, Organization, Team} from 'sentry/types';
 
 export interface BaseBadgeProps {
-  displayName: React.ReactNode;
   avatarProps?: Record<string, any>;
   avatarSize?: number;
   className?: string;
@@ -14,12 +13,17 @@ export interface BaseBadgeProps {
   // Hides the main display name
   hideAvatar?: boolean;
   hideName?: boolean;
+}
+
+interface AllBaseBadgeProps extends BaseBadgeProps {
+  displayName: React.ReactNode;
   organization?: Organization;
   project?: AvatarProject;
   team?: Team;
+  user?: AvatarUser;
 }
 
-const BaseBadge = memo(
+export const BaseBadge = memo(
   ({
     displayName,
     hideName = false,
@@ -28,20 +32,20 @@ const BaseBadge = memo(
     avatarSize = 24,
     description,
     team,
+    user,
     organization,
     project,
     className,
-  }: BaseBadgeProps) => (
+  }: AllBaseBadgeProps) => (
     <Wrapper className={className}>
       {!hideAvatar && (
-        <StyledAvatar
+        <Avatar
           {...avatarProps}
           size={avatarSize}
-          hideName={hideName}
           team={team}
+          user={user}
           organization={organization}
           project={project}
-          data-test-id="badge-styled-avatar"
         />
       )}
 
@@ -57,19 +61,13 @@ const BaseBadge = memo(
   )
 );
 
-export default BaseBadge;
-
 const Wrapper = styled('div')`
   display: flex;
+  gap: ${space(1)};
   align-items: center;
   flex-shrink: 0;
 `;
 
-const StyledAvatar = styled(Avatar)<{hideName: boolean}>`
-  margin-right: ${p => (p.hideName ? 0 : space(1))};
-  flex-shrink: 0;
-`;
-
 const DisplayNameAndDescription = styled('div')`
   display: flex;
   flex-direction: column;

+ 2 - 3
static/app/components/idBadge/getBadge.tsx

@@ -5,11 +5,10 @@ import ProjectBadge, {type ProjectBadgeProps} from './projectBadge';
 import {TeamBadge, type TeamBadgeProps} from './teamBadge';
 import UserBadge, {type UserBadgeProps} from './userBadge';
 
-type DisplayName = BaseBadgeProps['displayName'];
-
 interface AddedBaseBadgeProps {
-  displayName?: DisplayName;
+  displayName?: React.ReactNode;
 }
+
 interface GetOrganizationBadgeProps
   extends AddedBaseBadgeProps,
     Omit<BaseBadgeProps, 'displayName' | 'organization'>,

+ 2 - 2
static/app/components/idBadge/index.spec.tsx

@@ -17,7 +17,7 @@ describe('IdBadge', function () {
 
   it('renders the correct component when `team` property is passed', function () {
     render(<IdBadge team={TeamFixture()} />);
-    expect(screen.getByTestId('badge-styled-avatar')).toHaveTextContent('TS');
+    expect(screen.getByTestId('letter_avatar-avatar')).toHaveTextContent('TS');
     expect(screen.getByTestId('badge-display-name')).toHaveTextContent('#team-slug');
   });
 
@@ -28,7 +28,7 @@ describe('IdBadge', function () {
 
   it('renders the correct component when `organization` property is passed', function () {
     render(<IdBadge organization={OrganizationFixture()} />);
-    expect(screen.getByTestId('badge-styled-avatar')).toHaveTextContent('OS');
+    expect(screen.getByTestId('default-avatar')).toHaveTextContent('OS');
     expect(screen.getByTestId('badge-display-name')).toHaveTextContent('org-slug');
   });
 

+ 3 - 10
static/app/components/idBadge/memberBadge.spec.tsx

@@ -14,22 +14,15 @@ describe('MemberBadge', function () {
   });
 
   it('renders with link when member and orgId are supplied', function () {
-    render(<MemberBadge member={member} orgId="orgId" />, {context: routerContext});
+    render(<MemberBadge member={member} />, {context: routerContext});
 
     expect(screen.getByTestId('letter_avatar-avatar')).toBeInTheDocument();
     expect(screen.getByRole('link', {name: 'Foo Bar'})).toBeInTheDocument();
     expect(screen.getByText('foo@example.com')).toBeInTheDocument();
   });
 
-  it('does not use a link when useLink = false', function () {
-    render(<MemberBadge member={member} useLink={false} orgId="orgId" />);
-
-    expect(screen.queryByRole('link', {name: 'Foo Bar'})).not.toBeInTheDocument();
-    expect(screen.getByText('Foo Bar')).toBeInTheDocument();
-  });
-
-  it('does not use a link when orgId = null', function () {
-    render(<MemberBadge member={member} useLink />);
+  it('does not use a link when disableLink', function () {
+    render(<MemberBadge member={member} disableLink />);
 
     expect(screen.queryByRole('link', {name: 'Foo Bar'})).not.toBeInTheDocument();
     expect(screen.getByText('Foo Bar')).toBeInTheDocument();

+ 15 - 88
static/app/components/idBadge/memberBadge.tsx

@@ -1,21 +1,14 @@
-import styled from '@emotion/styled';
-import omit from 'lodash/omit';
-
-import UserAvatar from 'sentry/components/avatar/userAvatar';
-import type {LinkProps} from 'sentry/components/links/link';
-import Link from 'sentry/components/links/link';
-import {space} from 'sentry/styles/space';
 import type {AvatarUser, Member} from 'sentry/types';
+import useOrganization from 'sentry/utils/useOrganization';
+
+import UserBadge, {type UserBadgeProps} from './userBadge';
 
-export interface MemberBadgeProps {
+export interface MemberBadgeProps extends Omit<UserBadgeProps, 'user'> {
   member: Member;
-  avatarSize?: React.ComponentProps<typeof UserAvatar>['size'];
-  className?: string;
-  displayEmail?: string;
-  displayName?: React.ReactNode;
-  hideEmail?: boolean;
-  orgId?: string;
-  useLink?: boolean;
+  /**
+   * Do not link to the members page
+   */
+  disableLink?: boolean;
 }
 
 function getMemberUser(member: Member): AvatarUser {
@@ -32,82 +25,16 @@ function getMemberUser(member: Member): AvatarUser {
   };
 }
 
-function MemberBadge({
-  avatarSize = 24,
-  useLink = true,
-  hideEmail = false,
-  displayName,
-  displayEmail,
-  member,
-  orgId,
-  className,
-}: MemberBadgeProps) {
+function MemberBadge({member, disableLink, ...props}: MemberBadgeProps) {
   const user = getMemberUser(member);
-  const title =
-    displayName ||
-    user.name ||
-    user.email ||
-    user.username ||
-    user.ipAddress ||
-    // Because this can be used to render EventUser models, or User *interface*
-    // objects from serialized Event models. we try both ipAddress and ip_address.
-    user.ip_address;
-
-  return (
-    <StyledUserBadge className={className}>
-      <StyledAvatar user={user} size={avatarSize} />
-      <StyledNameAndEmail>
-        <StyledName
-          useLink={useLink && !!orgId}
-          hideEmail={hideEmail}
-          to={(member && orgId && `/settings/${orgId}/members/${member.id}/`) || ''}
-        >
-          {title}
-        </StyledName>
-        {!hideEmail && <StyledEmail>{displayEmail || user.email}</StyledEmail>}
-      </StyledNameAndEmail>
-    </StyledUserBadge>
-  );
-}
+  const org = useOrganization({allowNull: true});
 
-const StyledUserBadge = styled('div')`
-  display: flex;
-  align-items: center;
-`;
+  const membersUrl =
+    member && org && !disableLink
+      ? `/settings/${org.slug}/members/${member.id}/`
+      : undefined;
 
-const StyledNameAndEmail = styled('div')`
-  flex-shrink: 1;
-  min-width: 0;
-  line-height: 1;
-`;
-
-const StyledEmail = styled('div')`
-  font-size: 0.875em;
-  margin-top: ${space(0.25)};
-  color: ${p => p.theme.gray300};
-  ${p => p.theme.overflowEllipsis};
-`;
-
-interface NameProps {
-  hideEmail: boolean;
-  to: LinkProps['to'];
-  useLink: boolean;
-  children?: React.ReactNode;
+  return <UserBadge to={membersUrl} user={user} {...props} />;
 }
 
-const StyledName = styled(({useLink, to, ...props}: NameProps) => {
-  const forwardProps = omit(props, 'hideEmail');
-  return useLink ? <Link to={to} {...forwardProps} /> : <span {...forwardProps} />;
-})`
-  font-weight: ${(p: NameProps) => (p.hideEmail ? 'inherit' : 'bold')};
-  line-height: 1.15em;
-  ${p => p.theme.overflowEllipsis};
-`;
-
-const StyledAvatar = styled(UserAvatar)`
-  min-width: ${space(3)};
-  min-height: ${space(3)};
-  margin-right: ${space(1)};
-`;
-
 export default MemberBadge;

+ 1 - 1
static/app/components/idBadge/organizationBadge.spec.tsx

@@ -7,7 +7,7 @@ import OrganizationBadge from 'sentry/components/idBadge/organizationBadge';
 describe('OrganizationBadge', function () {
   it('renders with Avatar and organization name', function () {
     render(<OrganizationBadge organization={OrganizationFixture()} />);
-    expect(screen.getByTestId('badge-styled-avatar')).toBeInTheDocument();
+    expect(screen.getByTestId('default-avatar')).toBeInTheDocument();
     expect(screen.getByTestId('badge-display-name')).toHaveTextContent('org-slug');
   });
 });

+ 7 - 9
static/app/components/idBadge/organizationBadge.tsx

@@ -1,15 +1,13 @@
-import BadgeDisplayName from 'sentry/components/idBadge/badgeDisplayName';
-import BaseBadge from 'sentry/components/idBadge/baseBadge';
+import type {Organization} from 'sentry/types';
 
-type BaseBadgeProps = React.ComponentProps<typeof BaseBadge>;
-type Organization = NonNullable<BaseBadgeProps['organization']>;
+import BadgeDisplayName from './badgeDisplayName';
+import {BaseBadge, type BaseBadgeProps} from './baseBadge';
 
-export interface OrganizationBadgeProps
-  extends Partial<Omit<BaseBadgeProps, 'project' | 'organization' | 'team'>> {
-  // A full organization is not used, but required to satisfy types with
-  // withOrganization()
+export interface OrganizationBadgeProps extends BaseBadgeProps {
   organization: Organization;
-  // If true, will use default max-width, or specify one as a string
+  /**
+   * When true will default max-width, or specify one as a string
+   */
   hideOverflow?: boolean | string;
 }
 

+ 5 - 8
static/app/components/idBadge/projectBadge.tsx

@@ -1,20 +1,17 @@
 import {cloneElement} from 'react';
 import styled from '@emotion/styled';
 
-import BadgeDisplayName from 'sentry/components/idBadge/badgeDisplayName';
-import BaseBadge from 'sentry/components/idBadge/baseBadge';
 import type {LinkProps} from 'sentry/components/links/link';
 import Link from 'sentry/components/links/link';
+import type {AvatarProject} from 'sentry/types';
 import getPlatformName from 'sentry/utils/getPlatformName';
 import useOrganization from 'sentry/utils/useOrganization';
 
-type BaseBadgeProps = React.ComponentProps<typeof BaseBadge>;
-type Project = NonNullable<BaseBadgeProps['project']>;
+import BadgeDisplayName from './badgeDisplayName';
+import {BaseBadge, type BaseBadgeProps} from './baseBadge';
 
-export interface ProjectBadgeProps
-  extends Partial<Omit<BaseBadgeProps, 'project' | 'organization' | 'team'>> {
-  project: Project;
-  className?: string;
+export interface ProjectBadgeProps extends BaseBadgeProps {
+  project: AvatarProject;
   /**
    * If true, this component will not be a link to project details page
    */

+ 1 - 1
static/app/components/idBadge/teamBadge.spec.tsx

@@ -12,7 +12,7 @@ describe('TeamBadge', function () {
 
   it('renders with Avatar and team name', function () {
     render(<TeamBadge team={TeamFixture()} />);
-    expect(screen.getByTestId('badge-styled-avatar')).toBeInTheDocument();
+    expect(screen.getByTestId('letter_avatar-avatar')).toBeInTheDocument();
     expect(screen.getByText(/#team-slug/)).toBeInTheDocument();
   });
 

Some files were not shown because too many files changed in this diff