Skip to content

ref(ui): Simplify badges implementation #68946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 16, 2024
Merged

ref(ui): Simplify badges implementation #68946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc8df41
ref(ui): Simplify badges implementation
evanpurkhiser Apr 15, 2024
aa45093
:hammer_and_wrench: apply pre-commit fixes
getsantry[bot] Apr 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion static/app/components/idBadge/baseBadge.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
26 changes: 12 additions & 14 deletions static/app/components/idBadge/baseBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,27 @@ 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;
description?: React.ReactNode;
// 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,
Expand All @@ -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"
/>
)}

Expand All @@ -57,19 +61,13 @@ const BaseBadge = memo(
)
);

export default BaseBadge;

const Wrapper = styled('div')`
display: flex;
gap: ${space(1)};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk if this is properly replacing the margin-right that StyledAvatar had before.

The problem I'm seeing is that the replay breadcrumbItem has no spacing now:
SCR-20240416-jpqv

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should look like this:
SCR-20240416-jpyq

from https://sentry.sentry.io/replays/6745a47f9add410198126dd92a43983e/?query=&referrer=%2Freplays%2F&statsPeriod=14d&yAxis=count%28%29

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waiting to see the combo with #69009, will return to look again!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will have a merge conflict once #69009 merges

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;
Expand Down
5 changes: 2 additions & 3 deletions static/app/components/idBadge/getBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'>,
Expand Down
4 changes: 2 additions & 2 deletions static/app/components/idBadge/index.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

Expand All @@ -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');
});

Expand Down
13 changes: 3 additions & 10 deletions static/app/components/idBadge/memberBadge.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
103 changes: 15 additions & 88 deletions static/app/components/idBadge/memberBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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} />;
Comment on lines +32 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is amazing

}

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;
2 changes: 1 addition & 1 deletion static/app/components/idBadge/organizationBadge.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
16 changes: 7 additions & 9 deletions static/app/components/idBadge/organizationBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
}

Expand Down
13 changes: 5 additions & 8 deletions static/app/components/idBadge/projectBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion static/app/components/idBadge/teamBadge.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down
5 changes: 2 additions & 3 deletions static/app/components/idBadge/teamBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ import {useLegacyStore} from 'sentry/stores/useLegacyStore';
import type {Team} from 'sentry/types';

import BadgeDisplayName from './badgeDisplayName';
import BaseBadge, {type BaseBadgeProps} from './baseBadge';
import {BaseBadge, type BaseBadgeProps} from './baseBadge';

export interface TeamBadgeProps
extends Partial<Omit<BaseBadgeProps, 'project' | 'organization' | 'team'>> {
export interface TeamBadgeProps extends BaseBadgeProps {
team: Team;
/**
* When true will default max-width, or specify one as a string
Expand Down
Loading