Browse Source

ref(teamKeyTransaction): Use CompactSelect (#50791)

**Before ——**
<img width="238" alt="Screenshot 2023-06-12 at 6 16 11 PM"
src="https://github.com/getsentry/sentry/assets/44172267/ac04229b-fad2-4e17-843c-e8c6eea3d58c">
<img width="234" alt="Screenshot 2023-06-12 at 6 17 45 PM"
src="https://github.com/getsentry/sentry/assets/44172267/30776b66-5de7-40ad-b076-8204b58e01da">

**After ——**
<img width="194" alt="Screenshot 2023-06-12 at 6 15 45 PM"
src="https://github.com/getsentry/sentry/assets/44172267/7b8c5dca-4844-4174-a7c1-22ae2815d428">
<img width="197" alt="Screenshot 2023-06-12 at 6 17 20 PM"
src="https://github.com/getsentry/sentry/assets/44172267/ab8e1e49-3cc5-43b0-829b-3b7df2d752a9">
Vu Luong 1 year ago
parent
commit
4c15a4f639

+ 89 - 373
static/app/components/performance/teamKeyTransaction.tsx

@@ -1,416 +1,132 @@
-import {Component, Fragment} from 'react';
-import {createPortal} from 'react-dom';
-import {Manager, Popper, Reference} from 'react-popper';
+import {useCallback, useMemo} from 'react';
 import styled from '@emotion/styled';
 
-import MenuHeader from 'sentry/components/actions/menuHeader';
-import Checkbox from 'sentry/components/checkbox';
-import {GetActorPropsFn} from 'sentry/components/deprecatedDropdownMenu';
-import MenuItem from 'sentry/components/menuItem';
+import TeamAvatar from 'sentry/components/avatar/teamAvatar';
+import {CompactSelect, MultipleSelectProps} from 'sentry/components/compactSelect';
 import {TeamSelection} from 'sentry/components/performance/teamKeyTransactionsManager';
 import {t} from 'sentry/locale';
-import {space} from 'sentry/styles/space';
 import {Organization, Project, Team} from 'sentry/types';
-import {defined} from 'sentry/utils';
 import {trackAnalytics} from 'sentry/utils/analytics';
 import {MAX_TEAM_KEY_TRANSACTIONS} from 'sentry/utils/performance/constants';
 
-export type TitleProps = Partial<ReturnType<GetActorPropsFn>> & {
-  isOpen: boolean;
-  keyedTeams: Team[] | null;
-  disabled?: boolean;
-  initialValue?: number;
-};
-
-type Props = {
+type TeamKeyTransactionProps = Omit<
+  MultipleSelectProps<string>,
+  'multiple' | 'options' | 'value' | 'defaultValue' | 'onChange' | 'title'
+> & {
   counts: Map<string, number> | null;
-  error: string | null;
   handleToggleKeyTransaction: (selection: TeamSelection) => void;
-  isLoading: boolean;
   keyedTeams: Set<string> | null;
   organization: Organization;
   project: Project;
   teams: Team[];
-  title: React.ComponentType<TitleProps>;
   transactionName: string;
-  initialValue?: number;
 };
 
-type State = {
-  isOpen: boolean;
-};
-
-class TeamKeyTransaction extends Component<Props, State> {
-  state: State = {
-    isOpen: false,
-  };
-
-  componentDidUpdate(_props: Props, prevState: State) {
-    if (this.state.isOpen && prevState.isOpen === false) {
-      document.addEventListener('click', this.handleClickOutside, true);
-    }
-    if (this.state.isOpen === false && prevState.isOpen) {
-      document.removeEventListener('click', this.handleClickOutside, true);
-    }
-  }
-
-  componentWillUnmount() {
-    document.removeEventListener('click', this.handleClickOutside, true);
-  }
-
-  private menuEl: Element | null = null;
-
-  handleClickOutside = (event: MouseEvent) => {
-    if (!this.menuEl) {
-      return;
-    }
-    if (!(event.target instanceof Element)) {
-      return;
-    }
-    if (this.menuEl.contains(event.target)) {
-      return;
-    }
-    this.setState({isOpen: false});
-  };
-
-  toggleOpen = () => {
-    this.setState(({isOpen}) => ({isOpen: !isOpen}));
-  };
-
-  toggleSelection = (enabled: boolean, selection: TeamSelection) => () => {
-    const {handleToggleKeyTransaction, organization} = this.props;
-    const {action} = selection;
-    trackAnalytics('performance_views.team_key_transaction.set', {
-      organization,
-      action,
-    });
-
-    return enabled ? handleToggleKeyTransaction(selection) : undefined;
-  };
-
-  partitionTeams(counts: Map<string, number>, keyedTeams: Set<string>) {
-    const {teams, project} = this.props;
+function TeamKeyTransaction({
+  keyedTeams,
+  teams,
+  project,
+  counts,
+  handleToggleKeyTransaction,
+  transactionName,
+  organization,
+  ...props
+}: TeamKeyTransactionProps) {
+  const projectTeams = useMemo(() => new Set(project.teams.map(({id}) => id)), [project]);
+
+  const value = useMemo(
+    () => (keyedTeams ? [...projectTeams].filter(teamId => keyedTeams.has(teamId)) : []),
+    [keyedTeams, projectTeams]
+  );
 
+  const options = useMemo<MultipleSelectProps<string>['options']>(() => {
     const enabledTeams: Team[] = [];
     const disabledTeams: Team[] = [];
-    const noAccessTeams: Team[] = [];
 
-    const projectTeams = new Set(project.teams.map(({id}) => id));
     for (const team of teams) {
       if (!projectTeams.has(team.id)) {
-        noAccessTeams.push(team);
-      } else if (
+        continue;
+      }
+
+      if (
+        !keyedTeams ||
         keyedTeams.has(team.id) ||
+        !counts ||
         (counts.get(team.id) ?? 0) < MAX_TEAM_KEY_TRANSACTIONS
       ) {
         enabledTeams.push(team);
-      } else {
-        disabledTeams.push(team);
+        continue;
       }
-    }
-
-    return {
-      enabledTeams,
-      disabledTeams,
-      noAccessTeams,
-    };
-  }
-
-  renderMenuContent(counts: Map<string, number>, keyedTeams: Set<string>) {
-    const {teams, project, transactionName} = this.props;
-
-    const {enabledTeams, disabledTeams, noAccessTeams} = this.partitionTeams(
-      counts,
-      keyedTeams
-    );
-
-    const isMyTeamsEnabled = enabledTeams.length > 0;
-    const myTeamsHandler = this.toggleSelection(isMyTeamsEnabled, {
-      action: enabledTeams.length === keyedTeams.size ? 'unkey' : 'key',
-      teamIds: enabledTeams.map(({id}) => id),
-      project,
-      transactionName,
-    });
-
-    const hasTeamsWithAccess = enabledTeams.length + disabledTeams.length > 0;
-
-    return (
-      <DropdownContent>
-        {hasTeamsWithAccess && (
-          <Fragment>
-            <DropdownMenuHeader first>
-              {t('My Teams with Access')}
-              <ActionItem>
-                <Checkbox
-                  aria-label={t('My Teams with Access')}
-                  disabled={!isMyTeamsEnabled}
-                  checked={
-                    teams.length > keyedTeams.size && keyedTeams.size > 0
-                      ? 'indeterminate'
-                      : teams.length === keyedTeams.size
-                  }
-                  onChange={myTeamsHandler}
-                />
-              </ActionItem>
-            </DropdownMenuHeader>
-            {enabledTeams.map(team => (
-              <TeamKeyTransactionItem
-                key={team.slug}
-                team={team}
-                isKeyed={keyedTeams.has(team.id)}
-                disabled={false}
-                onSelect={this.toggleSelection(true, {
-                  action: keyedTeams.has(team.id) ? 'unkey' : 'key',
-                  teamIds: [team.id],
-                  project,
-                  transactionName,
-                })}
-              />
-            ))}
-            {disabledTeams.map(team => (
-              <TeamKeyTransactionItem
-                key={team.slug}
-                team={team}
-                isKeyed={keyedTeams.has(team.id)}
-                disabled
-                onSelect={this.toggleSelection(true, {
-                  action: keyedTeams.has(team.id) ? 'unkey' : 'key',
-                  teamIds: [team.id],
-                  project,
-                  transactionName,
-                })}
-              />
-            ))}
-          </Fragment>
-        )}
-        {noAccessTeams.length > 0 && (
-          <Fragment>
-            <DropdownMenuHeader first={!hasTeamsWithAccess}>
-              {t('My Teams without Access')}
-            </DropdownMenuHeader>
-            {noAccessTeams.map(team => (
-              <TeamKeyTransactionItem key={team.slug} team={team} disabled />
-            ))}
-          </Fragment>
-        )}
-      </DropdownContent>
-    );
-  }
-
-  renderMenu(): React.ReactPortal | null {
-    const {isLoading, counts, keyedTeams} = this.props;
 
-    if (isLoading || !defined(counts) || !defined(keyedTeams)) {
-      return null;
+      disabledTeams.push(team);
     }
 
-    const modifiers = [
+    return [
       {
-        name: 'hide',
-        enabled: false,
-      },
-      {
-        name: 'preventOverflow',
-        enabled: true,
-        options: {padding: 10},
+        label: t('My Teams'),
+        showToggleAllButton: enabledTeams.length > 1,
+        options: [
+          ...enabledTeams.map(team => ({
+            value: team.id,
+            label: `#${team.slug}`,
+            leadingItems: <TeamAvatar size={18} team={team} />,
+          })),
+          ...disabledTeams.map(team => ({
+            value: team.id,
+            label: `#${team.slug}`,
+            disabled: true,
+            leadingItems: <TeamAvatar size={18} team={team} />,
+            trailingItems: t('Max %s', MAX_TEAM_KEY_TRANSACTIONS),
+          })),
+        ],
       },
     ];
-
-    return createPortal(
-      <Popper placement="top" modifiers={modifiers}>
-        {({ref: popperRef, style, placement}) => (
-          <DropdownWrapper
-            ref={ref => {
-              (popperRef as Function)(ref);
-              this.menuEl = ref;
-            }}
-            style={style}
-            data-placement={placement}
-          >
-            {this.renderMenuContent(counts, keyedTeams)}
-          </DropdownWrapper>
-        )}
-      </Popper>,
-      document.body
-    );
-  }
-
-  render() {
-    const {isLoading, error, title: Title, keyedTeams, initialValue, teams} = this.props;
-    const {isOpen} = this.state;
-
-    const menu: React.ReactPortal | null = isOpen ? this.renderMenu() : null;
-
-    return (
-      <Manager>
-        <Reference>
-          {({ref}) => (
-            <StarWrapper ref={ref}>
-              <Title
-                isOpen={isOpen}
-                disabled={isLoading || Boolean(error)}
-                keyedTeams={
-                  keyedTeams ? teams.filter(({id}) => keyedTeams.has(id)) : null
-                }
-                initialValue={initialValue}
-                onClick={this.toggleOpen}
-              />
-            </StarWrapper>
-          )}
-        </Reference>
-        {menu}
-      </Manager>
-    );
-  }
-}
-
-type ItemProps = {
-  disabled: boolean;
-  team: Team;
-  isKeyed?: boolean;
-  onSelect?: () => void;
-};
-
-function TeamKeyTransactionItem({team, isKeyed, disabled, onSelect}: ItemProps) {
-  const id = `team_key_transaction_${team.slug}`;
+  }, [teams, counts, projectTeams, keyedTeams]);
+
+  const handleChange = useCallback<NonNullable<MultipleSelectProps<string>['onChange']>>(
+    opts => {
+      const selection = opts.map(opt => opt.value);
+      const keyed = selection.filter(id => !keyedTeams?.has(id));
+      const unkeyed = keyedTeams
+        ? [...keyedTeams].filter(id => !selection.includes(id))
+        : selection;
+
+      const action = keyed.length > 0 ? 'key' : 'unkey';
+      trackAnalytics('performance_views.team_key_transaction.set', {
+        organization,
+        action,
+      });
+
+      handleToggleKeyTransaction({
+        action,
+        teamIds: keyed.length > 0 ? keyed : unkeyed,
+        project,
+        transactionName,
+      });
+    },
+    [handleToggleKeyTransaction, keyedTeams, transactionName, organization, project]
+  );
 
   return (
-    <DropdownMenuItem
-      key={team.slug}
-      disabled={disabled}
-      onSelect={onSelect}
-      stopPropagation
-    >
-      <MenuItemContent id={id}>
-        {team.slug}
-        <ActionItem>
-          {!defined(isKeyed) ? null : disabled ? (
-            t('Max %s', MAX_TEAM_KEY_TRANSACTIONS)
-          ) : (
-            <Checkbox
-              onClick={e => e.stopPropagation()}
-              aria-labelledby={id}
-              checked={isKeyed}
-              onChange={onSelect}
-            />
-          )}
-        </ActionItem>
-      </MenuItemContent>
-    </DropdownMenuItem>
+    <Wrapper>
+      <CompactSelect
+        multiple
+        value={value}
+        onChange={handleChange}
+        options={options}
+        searchable={options.length > 8}
+        {...props}
+      />
+    </Wrapper>
   );
 }
 
-const StarWrapper = styled('div')`
-  display: flex;
-
-  /* Fixes Star when it’s filled and is wrapped around Tooltip */
-  & > span {
-    display: flex;
-  }
-`;
-
-const DropdownWrapper = styled('div')`
-  /* Adapted from the dropdown-menu class */
-  border: none;
-  border-radius: 2px;
-  box-shadow: 0 0 0 1px rgba(52, 60, 69, 0.2), 0 1px 3px rgba(70, 82, 98, 0.25);
-  background-clip: padding-box;
-  background-color: ${p => p.theme.background};
-  width: 220px;
-  overflow: visible;
-  z-index: ${p => p.theme.zIndex.tooltip};
-
-  &:before,
-  &:after {
-    width: 0;
-    height: 0;
-    content: '';
-    display: block;
-    position: absolute;
-    right: auto;
-  }
-
-  &:before {
-    border-left: 9px solid transparent;
-    border-right: 9px solid transparent;
-    left: calc(50% - 9px);
-    z-index: -2;
-  }
-
-  &:after {
-    border-left: 8px solid transparent;
-    border-right: 8px solid transparent;
-    left: calc(50% - 8px);
-    z-index: -1;
-  }
-
-  &[data-placement*='bottom'] {
-    margin-top: 9px;
-
-    &:before {
-      border-bottom: 9px solid ${p => p.theme.border};
-      top: -9px;
-    }
-
-    &:after {
-      border-bottom: 8px solid ${p => p.theme.background};
-      top: -8px;
-    }
+const Wrapper = styled('div')`
+  position: relative;
+  ul,
+  p {
+    margin: 0;
   }
-
-  &[data-placement*='top'] {
-    margin-bottom: 9px;
-
-    &:before {
-      border-top: 9px solid ${p => p.theme.border};
-      bottom: -9px;
-    }
-
-    &:after {
-      border-top: 8px solid ${p => p.theme.background};
-      bottom: -8px;
-    }
-  }
-`;
-
-const DropdownContent = styled('div')`
-  max-height: 250px;
-  pointer-events: auto;
-  overflow-y: auto;
-`;
-
-const DropdownMenuHeader = styled(MenuHeader)<{first?: boolean}>`
-  display: flex;
-  flex-direction: row;
-  justify-content: space-between;
-  align-items: center;
-  padding: ${space(1)} ${space(2)};
-
-  background: ${p => p.theme.backgroundSecondary};
-  ${p => p.first && 'border-radius: 2px'};
-`;
-
-const DropdownMenuItem = styled(MenuItem)`
-  font-size: ${p => p.theme.fontSizeMedium};
-
-  &:not(:last-child) {
-    border-bottom: 1px solid ${p => p.theme.innerBorder};
-  }
-`;
-
-const MenuItemContent = styled('div')`
-  display: flex;
-  flex-direction: row;
-  justify-content: space-between;
-  align-items: center;
-  width: 100%;
-`;
-
-const ActionItem = styled('span')`
-  display: flex;
-  align-items: center;
-  min-width: ${space(2)};
-  margin-left: ${space(1)};
 `;
 
 export default TeamKeyTransaction;

+ 6 - 8
static/app/utils/discover/fieldRenderers.tsx

@@ -653,14 +653,12 @@ const SPECIAL_FIELDS: SpecialFields = {
   team_key_transaction: {
     sortField: null,
     renderFunc: (data, {organization}) => (
-      <Container>
-        <TeamKeyTransactionField
-          isKeyTransaction={(data.team_key_transaction ?? 0) !== 0}
-          organization={organization}
-          projectSlug={data.project}
-          transactionName={data.transaction}
-        />
-      </Container>
+      <TeamKeyTransactionField
+        isKeyTransaction={(data.team_key_transaction ?? 0) !== 0}
+        organization={organization}
+        projectSlug={data.project}
+        transactionName={data.transaction}
+      />
     ),
   },
   'trend_percentage()': {

+ 25 - 91
static/app/utils/discover/teamKeyTransactionField.spec.tsx

@@ -53,12 +53,9 @@ describe('TeamKeyTransactionField', function () {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
 
-    const [allTeamsCheckbox, teamOneCheckbox, teamTwoCheckbox] =
-      screen.getAllByRole('checkbox');
-
-    expect(allTeamsCheckbox).toBeChecked();
-    expect(teamOneCheckbox).toBeChecked();
-    expect(teamTwoCheckbox).toBeChecked();
+    const [teamOneOption, teamTwoOption] = screen.getAllByRole('option');
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'true');
+    expect(teamTwoOption).toHaveAttribute('aria-selected', 'true');
   });
 
   it('renders with some teams checked', async function () {
@@ -98,12 +95,9 @@ describe('TeamKeyTransactionField', function () {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
 
-    const [allTeamsCheckbox, teamOneCheckbox, teamTwoCheckbox] =
-      screen.getAllByRole('checkbox');
-
-    expect(allTeamsCheckbox).not.toBeChecked();
-    expect(teamOneCheckbox).toBeChecked();
-    expect(teamTwoCheckbox).not.toBeChecked();
+    const [teamOneOption, teamTwoOption] = screen.getAllByRole('option');
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'true');
+    expect(teamTwoOption).toHaveAttribute('aria-selected', 'false');
   });
 
   it('renders with no teams checked', async function () {
@@ -140,12 +134,9 @@ describe('TeamKeyTransactionField', function () {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
 
-    const [allTeamsCheckbox, teamOneCheckbox, teamTwoCheckbox] =
-      screen.getAllByRole('checkbox');
-
-    expect(allTeamsCheckbox).not.toBeChecked();
-    expect(teamOneCheckbox).not.toBeChecked();
-    expect(teamTwoCheckbox).not.toBeChecked();
+    const [teamOneOption, teamTwoOption] = screen.getAllByRole('option');
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'false');
+    expect(teamTwoOption).toHaveAttribute('aria-selected', 'false');
   });
 
   it('should be able to check one team', async function () {
@@ -190,18 +181,16 @@ describe('TeamKeyTransactionField', function () {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
 
-    const [_allTeamsCheckbox, teamOneCheckbox, _teamTwoCheckbox] =
-      screen.getAllByRole('checkbox');
+    const teamOneOption = screen.getAllByRole('option')[0];
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'false');
 
-    expect(teamOneCheckbox).not.toBeChecked();
-
-    await userEvent.click(teamOneCheckbox);
+    await userEvent.click(teamOneOption);
     expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
     await waitFor(() => {
       expect(screen.getByRole('button', {name: 'Toggle star for team'})).toBeEnabled();
     });
 
-    expect(teamOneCheckbox).toBeChecked();
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'true');
   });
 
   it('should be able to uncheck one team', async function () {
@@ -246,18 +235,16 @@ describe('TeamKeyTransactionField', function () {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
 
-    const [_allTeamsCheckbox, teamOneCheckbox, _teamTwoCheckbox] =
-      screen.getAllByRole('checkbox');
-
-    expect(teamOneCheckbox).toBeChecked();
+    const teamOneOption = screen.getAllByRole('option')[0];
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'true');
 
-    await userEvent.click(teamOneCheckbox);
+    await userEvent.click(teamOneOption);
     expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
     await waitFor(() => {
       expect(screen.getByRole('button', {name: 'Toggle star for team'})).toBeEnabled();
     });
 
-    expect(teamOneCheckbox).not.toBeChecked();
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'false');
   });
 
   it('should be able to check all with my teams', async function () {
@@ -305,20 +292,16 @@ describe('TeamKeyTransactionField', function () {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
 
-    const [allTeamsCheckbox, teamOneCheckbox, teamTwoCheckbox] =
-      screen.getAllByRole('checkbox');
-
-    expect(allTeamsCheckbox).not.toBeChecked();
-    await userEvent.click(allTeamsCheckbox);
+    await userEvent.click(screen.getByRole('button', {name: 'Select All in My Teams'}));
 
     expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
     await waitFor(() => {
       expect(screen.getByRole('button', {name: 'Toggle star for team'})).toBeEnabled();
     });
 
-    expect(allTeamsCheckbox).toBeChecked();
-    expect(teamOneCheckbox).toBeChecked();
-    expect(teamTwoCheckbox).toBeChecked();
+    const [teamOneOption, teamTwoOption] = screen.getAllByRole('option');
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'true');
+    expect(teamTwoOption).toHaveAttribute('aria-selected', 'true');
   });
 
   it('should be able to uncheck all with my teams', async function () {
@@ -366,64 +349,15 @@ describe('TeamKeyTransactionField', function () {
 
     await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
 
-    const [allTeamsCheckbox, teamOneCheckbox, teamTwoCheckbox] =
-      screen.getAllByRole('checkbox');
-
-    expect(allTeamsCheckbox).toBeChecked();
-    await userEvent.click(allTeamsCheckbox);
+    await userEvent.click(screen.getByRole('button', {name: 'Unselect All in My Teams'}));
 
     expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
     await waitFor(() => {
       expect(screen.getByRole('button', {name: 'Toggle star for team'})).toBeEnabled();
     });
 
-    expect(allTeamsCheckbox).not.toBeChecked();
-    expect(teamOneCheckbox).not.toBeChecked();
-    expect(teamTwoCheckbox).not.toBeChecked();
-  });
-
-  it('should render teams without access separately', async function () {
-    const myTeams = [...teams, TestStubs.Team({id: '3', slug: 'team3', name: 'Team 3'})];
-    act(() => {
-      TeamStore.loadInitialData(myTeams);
-    });
-
-    MockApiClient.addMockResponse({
-      method: 'GET',
-      url: `/organizations/${organization.slug}/key-transactions-list/`,
-      body: myTeams.map(({id}) => ({
-        team: id,
-        count: 0,
-        keyed: [],
-      })),
-    });
-
-    render(
-      <TeamKeyTransactionManager.Provider
-        organization={organization}
-        teams={myTeams}
-        selectedTeams={['myteams']}
-      >
-        <TeamKeyTransactionField
-          isKeyTransaction
-          organization={organization}
-          projectSlug={project.slug}
-          transactionName="transaction"
-        />
-      </TeamKeyTransactionManager.Provider>
-    );
-
-    await waitFor(() => {
-      expect(screen.getByRole('button', {name: 'Toggle star for team'})).toBeEnabled();
-    });
-
-    await userEvent.click(screen.getByRole('button', {name: 'Toggle star for team'}));
-
-    expect(screen.getByText('My Teams with Access')).toBeInTheDocument();
-    expect(screen.getByText('My Teams without Access')).toBeInTheDocument();
-
-    // Only renders checkboxes for teams with access
-    expect(screen.getAllByRole('checkbox')).toHaveLength(3);
-    expect(screen.getByText('team3')).toBeInTheDocument();
+    const [teamOneOption, teamTwoOption] = screen.getAllByRole('option');
+    expect(teamOneOption).toHaveAttribute('aria-selected', 'false');
+    expect(teamTwoOption).toHaveAttribute('aria-selected', 'false');
   });
 });

+ 40 - 37
static/app/utils/discover/teamKeyTransactionField.tsx

@@ -1,7 +1,5 @@
 import {Button} from 'sentry/components/button';
-import TeamKeyTransaction, {
-  TitleProps,
-} from 'sentry/components/performance/teamKeyTransaction';
+import TeamKeyTransaction from 'sentry/components/performance/teamKeyTransaction';
 import * as TeamKeyTransactionManager from 'sentry/components/performance/teamKeyTransactionsManager';
 import {Tooltip} from 'sentry/components/tooltip';
 import {IconStar} from 'sentry/icons';
@@ -10,34 +8,6 @@ import {Organization, Project} from 'sentry/types';
 import {defined} from 'sentry/utils';
 import withProjects from 'sentry/utils/withProjects';
 
-function TitleStar({isOpen, keyedTeams, initialValue, ...props}: TitleProps) {
-  const keyedTeamsCount = keyedTeams?.length ?? initialValue ?? 0;
-  const star = (
-    <IconStar
-      color={keyedTeamsCount ? 'yellow400' : 'gray200'}
-      isSolid={keyedTeamsCount > 0}
-      data-test-id="team-key-transaction-column"
-    />
-  );
-
-  const button = (
-    <Button
-      {...props}
-      icon={star}
-      borderless
-      size="zero"
-      aria-label={t('Toggle star for team')}
-    />
-  );
-
-  if (!isOpen && keyedTeams?.length) {
-    const teamSlugs = keyedTeams.map(({slug}) => slug).join(', ');
-    return <Tooltip title={teamSlugs}>{button}</Tooltip>;
-  }
-
-  return button;
-}
-
 type BaseProps = {
   isKeyTransaction: boolean;
   organization: Organization;
@@ -55,18 +25,50 @@ function TeamKeyTransactionField({
   getKeyedTeams,
   project,
   transactionName,
+  error,
+  isLoading,
   ...props
 }: Props) {
   const keyedTeams = getKeyedTeams(project.id, transactionName);
+  const keyedTeamsCount = keyedTeams?.size ?? Number(isKeyTransaction);
+  const disabled = isLoading || !!error;
 
   return (
     <TeamKeyTransaction
+      size="sm"
+      offset={[-12, 8]}
       counts={counts}
       keyedTeams={keyedTeams}
-      title={TitleStar}
       project={project}
       transactionName={transactionName}
-      initialValue={Number(isKeyTransaction)}
+      trigger={(triggerProps, isOpen) => (
+        <Tooltip
+          disabled={disabled || isOpen}
+          title={
+            !isOpen && keyedTeams?.size
+              ? project.teams
+                  .filter(team => keyedTeams.has(team.id))
+                  .map(({slug}) => slug)
+                  .join(', ')
+              : null
+          }
+        >
+          <Button
+            {...triggerProps}
+            disabled={disabled}
+            borderless
+            size="zero"
+            icon={
+              <IconStar
+                color={keyedTeamsCount ? 'yellow400' : 'gray200'}
+                isSolid={keyedTeamsCount > 0}
+                data-test-id="team-key-transaction-column"
+              />
+            }
+            aria-label={t('Toggle star for team')}
+          />
+        </Tooltip>
+      )}
       {...props}
     />
   );
@@ -92,11 +94,12 @@ function TeamKeyTransactionFieldWrapper({
   // with no interactions.
   if (!defined(project) || !defined(transactionName)) {
     return (
-      <TitleStar
-        isOpen={false}
+      <Button
         disabled
-        keyedTeams={null}
-        initialValue={Number(isKeyTransaction)}
+        borderless
+        size="zero"
+        icon={<IconStar color="gray100" />}
+        aria-label={t('Toggle star for team')}
       />
     );
   }

+ 67 - 47
static/app/views/performance/transactionSummary/teamKeyTransactionButton.spec.jsx

@@ -7,8 +7,10 @@ import {MAX_TEAM_KEY_TRANSACTIONS} from 'sentry/utils/performance/constants';
 import TeamKeyTransactionButton from 'sentry/views/performance/transactionSummary/teamKeyTransactionButton';
 
 async function clickTeamKeyTransactionDropdown() {
-  await waitFor(() => expect(screen.getByRole('button')).toBeEnabled());
-  await userEvent.click(screen.getByRole('button'));
+  await waitFor(() =>
+    expect(screen.getByRole('button', {expanded: false})).toBeEnabled()
+  );
+  await userEvent.click(screen.getByRole('button', {expanded: false}));
 }
 
 describe('TeamKeyTransactionButton', function () {
@@ -81,12 +83,15 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    // header should show the checked state
-    expect(screen.getByRole('checkbox', {name: 'My Teams with Access'})).toBeChecked();
-
     // all teams should be checked
-    expect(screen.getByRole('checkbox', {name: teams[0].slug})).toBeChecked();
-    expect(screen.getByRole('checkbox', {name: teams[1].slug})).toBeChecked();
+    expect(screen.getByRole('option', {name: `#${teams[0].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
+    expect(screen.getByRole('option', {name: `#${teams[1].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
   });
 
   it('renders with some teams checked', async function () {
@@ -113,14 +118,15 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    // header should show the indeterminate state
-    expect(
-      screen.getByRole('checkbox', {name: 'My Teams with Access'})
-    ).toBePartiallyChecked();
-
     // only team 1 should be checked
-    expect(screen.getByRole('checkbox', {name: teams[0].slug})).toBeChecked();
-    expect(screen.getByRole('checkbox', {name: teams[1].slug})).not.toBeChecked();
+    expect(screen.getByRole('option', {name: `#${teams[0].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
+    expect(screen.getByRole('option', {name: `#${teams[1].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'false'
+    );
   });
 
   it('renders with no teams checked', async function () {
@@ -144,14 +150,15 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    // header should show the unchecked state
-    expect(
-      screen.getByRole('checkbox', {name: 'My Teams with Access'})
-    ).not.toBeChecked();
-
     // all teams should be unchecked
-    expect(screen.getByRole('checkbox', {name: teams[0].slug})).not.toBeChecked();
-    expect(screen.getByRole('checkbox', {name: teams[1].slug})).not.toBeChecked();
+    expect(screen.getByRole('option', {name: `#${teams[0].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'false'
+    );
+    expect(screen.getByRole('option', {name: `#${teams[1].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'false'
+    );
   });
 
   it('should be able to check one team', async function () {
@@ -185,7 +192,7 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    await userEvent.click(screen.getByRole('checkbox', {name: teams[0].slug}));
+    await userEvent.click(screen.getByRole('option', {name: `#${teams[0].slug}`}));
     expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
   });
 
@@ -220,7 +227,7 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    await userEvent.click(screen.getByRole('checkbox', {name: teams[0].slug}));
+    await userEvent.click(screen.getByRole('option', {name: `#${teams[0].slug}`}));
     expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
   });
 
@@ -258,13 +265,17 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    await userEvent.click(screen.getByRole('checkbox', {name: 'My Teams with Access'}));
+    await userEvent.click(screen.getByRole('button', {name: 'Select All in My Teams'}));
 
     // all teams should be checked now
-    await waitFor(() => {
-      expect(screen.getByRole('checkbox', {name: teams[0].slug})).toBeChecked();
-      expect(screen.getByRole('checkbox', {name: teams[1].slug})).toBeChecked();
-    });
+    expect(screen.getByRole('option', {name: `#${teams[0].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
+    expect(screen.getByRole('option', {name: `#${teams[1].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
 
     expect(postTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
   });
@@ -303,13 +314,17 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    await userEvent.click(screen.getByRole('checkbox', {name: 'My Teams with Access'}));
+    await userEvent.click(screen.getByRole('button', {name: 'Unselect All in My Teams'}));
 
-    // all teams should be unchecked now
-    await waitFor(() => {
-      expect(screen.getByRole('checkbox', {name: teams[0].slug})).not.toBeChecked();
-      expect(screen.getByRole('checkbox', {name: teams[1].slug})).not.toBeChecked();
-    });
+    // all teams should be checked now
+    expect(screen.getByRole('option', {name: `#${teams[0].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'false'
+    );
+    expect(screen.getByRole('option', {name: `#${teams[1].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'false'
+    );
 
     expect(deleteTeamKeyTransactionsMock).toHaveBeenCalledTimes(1);
   });
@@ -338,17 +353,15 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    expect(
-      screen.getByRole('button', {
-        name: `${teams[0].slug} Max ${MAX_TEAM_KEY_TRANSACTIONS}`,
-      })
-    ).toBeInTheDocument();
-
-    expect(
-      screen.getByRole('button', {
-        name: `${teams[1].slug} Max ${MAX_TEAM_KEY_TRANSACTIONS}`,
-      })
-    ).toBeInTheDocument();
+    expect(screen.getByRole('option', {name: `#${teams[0].slug}`})).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
+
+    expect(screen.getByRole('option', {name: `#${teams[1].slug}`})).toHaveAttribute(
+      'aria-disabled',
+      'true'
+    );
   });
 
   it('renders keyed as checked even if count is maxed', async function () {
@@ -378,7 +391,14 @@ describe('TeamKeyTransactionButton', function () {
 
     await clickTeamKeyTransactionDropdown();
 
-    expect(screen.getByRole('checkbox', {name: teams[0].slug})).toBeChecked();
-    expect(screen.getByRole('checkbox', {name: teams[1].slug})).toBeChecked();
+    expect(screen.getByRole('option', {name: `#${teams[0].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
+
+    expect(screen.getByRole('option', {name: `#${teams[1].slug}`})).toHaveAttribute(
+      'aria-selected',
+      'true'
+    );
   });
 });

+ 47 - 36
static/app/views/performance/transactionSummary/teamKeyTransactionButton.tsx

@@ -1,9 +1,5 @@
-import {Component} from 'react';
-
 import {Button} from 'sentry/components/button';
-import TeamKeyTransactionComponent, {
-  TitleProps,
-} from 'sentry/components/performance/teamKeyTransaction';
+import TeamKeyTransactionComponent from 'sentry/components/performance/teamKeyTransaction';
 import * as TeamKeyTransactionManager from 'sentry/components/performance/teamKeyTransactionsManager';
 import {Tooltip} from 'sentry/components/tooltip';
 import {IconStar} from 'sentry/icons';
@@ -14,34 +10,6 @@ import EventView from 'sentry/utils/discover/eventView';
 import {useTeams} from 'sentry/utils/useTeams';
 import withProjects from 'sentry/utils/withProjects';
 
-/**
- * This can't be a function component because `TeamKeyTransaction` uses
- * `DropdownControl` which in turn uses passes a ref to this component.
- */
-class TitleButton extends Component<TitleProps> {
-  render() {
-    const {isOpen, keyedTeams, ...props} = this.props;
-    const keyedTeamsCount = keyedTeams?.length ?? 0;
-    const button = (
-      <Button
-        {...props}
-        size="sm"
-        icon={keyedTeamsCount ? <IconStar color="yellow400" isSolid /> : <IconStar />}
-      >
-        {keyedTeamsCount
-          ? tn('Starred for Team', 'Starred for Teams', keyedTeamsCount)
-          : t('Star for Team')}
-      </Button>
-    );
-
-    if (!isOpen && keyedTeams?.length) {
-      const teamSlugs = keyedTeams.map(({slug}) => slug).join(', ');
-      return <Tooltip title={teamSlugs}>{button}</Tooltip>;
-    }
-    return button;
-  }
-}
-
 type BaseProps = {
   organization: Organization;
   transactionName: string;
@@ -57,16 +25,51 @@ function TeamKeyTransactionButton({
   getKeyedTeams,
   project,
   transactionName,
+  isLoading,
+  error,
   ...props
 }: Props) {
   const keyedTeams = getKeyedTeams(project.id, transactionName);
+  const keyedTeamsCount = keyedTeams?.size ?? 0;
+  const disabled = isLoading || !!error;
+
   return (
     <TeamKeyTransactionComponent
       counts={counts}
       keyedTeams={keyedTeams}
-      title={TitleButton}
       project={project}
       transactionName={transactionName}
+      offset={8}
+      size="md"
+      trigger={(triggerProps, isOpen) => (
+        <Tooltip
+          disabled={disabled || isOpen}
+          title={
+            keyedTeams?.size
+              ? project.teams
+                  .filter(team => keyedTeams.has(team.id))
+                  .map(({slug}) => slug)
+                  .join(', ')
+              : null
+          }
+        >
+          <Button
+            {...triggerProps}
+            disabled={disabled}
+            size="sm"
+            icon={
+              <IconStar
+                isSolid={!!keyedTeamsCount}
+                color={keyedTeamsCount ? 'yellow400' : 'subText'}
+              />
+            }
+          >
+            {keyedTeamsCount
+              ? tn('Starred for Team', 'Starred for Teams', keyedTeamsCount)
+              : t('Star for Team')}
+          </Button>
+        </Tooltip>
+      )}
       {...props}
     />
   );
@@ -86,13 +89,21 @@ function TeamKeyTransactionButtonWrapper({
   const {teams, initiallyLoaded} = useTeams({provideUserTeams: true});
 
   if (eventView.project.length !== 1) {
-    return <TitleButton isOpen={false} disabled keyedTeams={null} />;
+    return (
+      <Button disabled size="sm" icon={<IconStar />}>
+        {t('Star for Team')}
+      </Button>
+    );
   }
 
   const projectId = String(eventView.project[0]);
   const project = projects.find(proj => proj.id === projectId);
   if (!defined(project)) {
-    return <TitleButton isOpen={false} disabled keyedTeams={null} />;
+    return (
+      <Button disabled size="sm" icon={<IconStar />}>
+        {t('Star for Team')}
+      </Button>
+    );
   }
 
   return (

+ 1 - 1
static/app/views/performance/transactionSummary/transactionOverview/index.spec.tsx

@@ -812,7 +812,7 @@ describe('Performance > TransactionSummary', function () {
       // Click the key transaction button
       await userEvent.click(screen.getByRole('button', {name: 'Star for Team'}));
 
-      await userEvent.click(screen.getByText('team1'));
+      await userEvent.click(screen.getByRole('option', {name: '#team1'}));
 
       // Ensure request was made.
       expect(mockUpdate).toHaveBeenCalled();