From 3b4a86a6a4dca88dfdcc840653261f63e98df2d6 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Wed, 1 Jan 2025 10:06:46 -0500 Subject: [PATCH] feat: improve 'Claimed' column UX in datatable (#2986) --- .eslintrc.yaml | 1 + app/Platform/Actions/BuildGameListAction.php | 1 + .../Concerns/BuildsGameListQueries.php | 11 +- app/Platform/Data/GameClaimantData.php | 28 +++++ app/Platform/Data/GameData.php | 9 ++ lang/en_US.json | 1 - .../components/UserAvatar/UserAvatar.tsx | 3 +- .../UserAvatarStack/UserAvatarStack.test.tsx | 118 ++++++++++++++++++ .../UserAvatarStack/UserAvatarStack.tsx | 92 ++++++++++++++ .../components/UserAvatarStack/index.ts | 1 + .../common/models/base-avatar-props.model.ts | 10 +- .../AllGamesDataTable/useColumnDefinitions.ts | 2 +- .../useColumnDefinitions.ts | 2 +- .../GameListDataTable.test.tsx | 19 ++- .../HubGamesDataTable/useColumnDefinitions.ts | 3 +- .../useColumnDefinitions.ts | 2 +- ...uildHasActiveOrInReviewClaimsColumnDef.tsx | 33 +++-- .../js/test/factories/createGameClaimant.ts | 9 ++ resources/js/test/factories/index.ts | 1 + resources/js/types/generated.d.ts | 5 + 20 files changed, 317 insertions(+), 34 deletions(-) create mode 100644 app/Platform/Data/GameClaimantData.php create mode 100644 resources/js/common/components/UserAvatarStack/UserAvatarStack.test.tsx create mode 100644 resources/js/common/components/UserAvatarStack/UserAvatarStack.tsx create mode 100644 resources/js/common/components/UserAvatarStack/index.ts create mode 100644 resources/js/test/factories/createGameClaimant.ts diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 7a63a451f4..0f2e330b52 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -106,6 +106,7 @@ rules: "baseCommandListClassName", "containerClassName", "gameTitleClassName", + "imgClassName", "wrapperClassName", ], "plugins": ["prettier-plugin-tailwindcss"] diff --git a/app/Platform/Actions/BuildGameListAction.php b/app/Platform/Actions/BuildGameListAction.php index 4c203b87d0..84fb977451 100644 --- a/app/Platform/Actions/BuildGameListAction.php +++ b/app/Platform/Actions/BuildGameListAction.php @@ -93,6 +93,7 @@ public function execute( game: GameData::from($game)->include( 'achievementsPublished', 'badgeUrl', + 'claimants', 'hasActiveOrInReviewClaims', 'lastUpdated', 'numVisibleLeaderboards', diff --git a/app/Platform/Concerns/BuildsGameListQueries.php b/app/Platform/Concerns/BuildsGameListQueries.php index 9084e85903..0b6322ed09 100644 --- a/app/Platform/Concerns/BuildsGameListQueries.php +++ b/app/Platform/Concerns/BuildsGameListQueries.php @@ -34,7 +34,16 @@ private function buildBaseQuery( ?User $user = null, ?int $targetId = null, ): Builder { - $query = Game::with(['system']) + $query = Game::query() + ->with([ + 'system', + 'achievementSetClaims' => function ($query) { + $query->activeOrInReview()->with(['user' => function ($query) { + // Only select the fields we need for the UserData DTO. + $query->select(['ID', 'User', 'display_name', 'Permissions']); + }]); + }, + ]) ->withLastAchievementUpdate() ->addSelect(['GameData.*']) ->addSelect([ diff --git a/app/Platform/Data/GameClaimantData.php b/app/Platform/Data/GameClaimantData.php new file mode 100644 index 0000000000..7a577ab447 --- /dev/null +++ b/app/Platform/Data/GameClaimantData.php @@ -0,0 +1,28 @@ + */ + public Lazy|array $claimants, ) { } @@ -50,6 +53,12 @@ public static function fromGame(Game $game): self numVisibleLeaderboards: Lazy::create(fn () => $game->num_visible_leaderboards ?? 0), numUnresolvedTickets: Lazy::create(fn () => $game->num_unresolved_tickets ?? 0), hasActiveOrInReviewClaims: Lazy::create(fn () => $game->has_active_or_in_review_claims ?? false), + claimants: Lazy::create(fn () => $game->achievementSetClaims->map( + fn ($claim) => GameClaimantData::fromUser( + $claim->user, + $claim->ClaimType === ClaimType::Primary ? 'primary' : 'collaboration' + ) + )->all()), ); } } diff --git a/lang/en_US.json b/lang/en_US.json index 35f680f8e5..a13bafb2cb 100644 --- a/lang/en_US.json +++ b/lang/en_US.json @@ -211,7 +211,6 @@ "Notification type": "Notification type", "Notifications": "Notifications", "Notify me on the site": "Notify me on the site", - "One or more developers are currently working on this game.": "One or more developers are currently working on this game.", "Only people I follow can message me or post on my wall": "Only people I follow can message me or post on my wall", "Only png, jpeg, and gif files are supported.": "Only png, jpeg, and gif files are supported.", "Open Game": "Open Game", diff --git a/resources/js/common/components/UserAvatar/UserAvatar.tsx b/resources/js/common/components/UserAvatar/UserAvatar.tsx index a135528524..b37aea0a01 100644 --- a/resources/js/common/components/UserAvatar/UserAvatar.tsx +++ b/resources/js/common/components/UserAvatar/UserAvatar.tsx @@ -9,6 +9,7 @@ type UserAvatarProps = BaseAvatarProps & App.Data.User; export const UserAvatar: FC = ({ displayName, deletedAt, + imgClassName, hasTooltip = true, showImage = true, showLabel = true, @@ -33,7 +34,7 @@ export const UserAvatar: FC = ({ height={size} src={`http://media.retroachievements.org/UserPic/${displayName}.png`} alt={displayName ?? 'Deleted User'} - className="rounded-sm" + className={cn('rounded-sm', imgClassName)} /> ) : null} diff --git a/resources/js/common/components/UserAvatarStack/UserAvatarStack.test.tsx b/resources/js/common/components/UserAvatarStack/UserAvatarStack.test.tsx new file mode 100644 index 0000000000..2c31ab10b9 --- /dev/null +++ b/resources/js/common/components/UserAvatarStack/UserAvatarStack.test.tsx @@ -0,0 +1,118 @@ +import userEvent from '@testing-library/user-event'; + +import { createAuthenticatedUser } from '@/common/models'; +import { render, screen, waitFor } from '@/test'; +import { createUser } from '@/test/factories'; + +import { UserAvatarStack } from './UserAvatarStack'; + +describe('Component: UserAvatarStack', () => { + it('renders without crashing', () => { + // ARRANGE + const { container } = render(); + + // ASSERT + expect(container).toBeTruthy(); + }); + + it('given no users are provided, renders nothing', () => { + // ARRANGE + render(); + + // ASSERT + expect(screen.queryByRole('list')).not.toBeInTheDocument(); + }); + + it('given a list of users is provided under the max visible limit, shows all users', () => { + // ARRANGE + const users = [createUser(), createUser(), createUser()]; + + render(); + + // ASSERT + const avatarList = screen.getByRole('list'); + // eslint-disable-next-line testing-library/no-node-access -- this is fine here, the count of nodes is relevant + expect(avatarList.children).toHaveLength(3); + }); + + it('given more users than the max visible limit, shows the overflow indicator', () => { + // ARRANGE + const users = [ + createUser(), + createUser(), + createUser(), + createUser(), + createUser(), + createUser(), // !! 6th user + ]; + + render(); + + // ASSERT + const avatarList = screen.getByRole('list'); + // eslint-disable-next-line testing-library/no-node-access -- this is fine here, the count of nodes is relevant + expect(avatarList.children).toHaveLength(5); // !! 4 avatars + overflow + expect(screen.getByText(/\+2/i)).toBeVisible(); + }); + + it('given a size prop of 24, applies the correct size classes', () => { + // ARRANGE + const users = [ + createUser(), + createUser(), + createUser(), + createUser(), + createUser(), + createUser(), // !! 6th user + ]; + + render(); + + // ASSERT + const overflowIndicator = screen.getByTestId('overflow-indicator'); + expect(overflowIndicator).toHaveClass('size-6'); + }); + + it('given a size prop of 28, applies the correct size classes', () => { + // ARRANGE + const users = [ + createUser(), + createUser(), + createUser(), + createUser(), + createUser(), + createUser(), // !! 6th user + ]; + + render(); + + // ASSERT + const overflowIndicator = screen.getByTestId('overflow-indicator'); + expect(overflowIndicator).toHaveClass('size-7'); + }); + + it('given the user is using a non-English locale, formats the overflow list correctly', async () => { + // ARRANGE + const users = [ + createUser(), + createUser(), + createUser(), + createUser(), + createUser({ displayName: 'Scott' }), + createUser({ displayName: 'TheMysticalOne' }), + createUser({ displayName: 'Jamiras' }), + ]; + + render(, { + pageProps: { auth: { user: createAuthenticatedUser({ locale: 'pt_BR' }) } }, // !! + }); + + // ACT + await userEvent.hover(screen.getByText(/\+3/i)); + + // ASSERT + await waitFor(() => { + expect(screen.getAllByText(/Jamiras, Scott e TheMysticalOne/i)[0]).toBeVisible(); + }); + }); +}); diff --git a/resources/js/common/components/UserAvatarStack/UserAvatarStack.tsx b/resources/js/common/components/UserAvatarStack/UserAvatarStack.tsx new file mode 100644 index 0000000000..8dea7459fe --- /dev/null +++ b/resources/js/common/components/UserAvatarStack/UserAvatarStack.tsx @@ -0,0 +1,92 @@ +import { type FC, useId } from 'react'; + +import { usePageProps } from '@/common/hooks/usePageProps'; +import type { AuthenticatedUser, AvatarSize } from '@/common/models'; +import { cn } from '@/common/utils/cn'; + +import { BaseTooltip, BaseTooltipContent, BaseTooltipTrigger } from '../+vendor/BaseTooltip'; +import { UserAvatar } from '../UserAvatar'; + +interface UserAvatarStackProps { + users: App.Data.User[]; + + size?: AvatarSize; + + /** + * Maximum number of avatars to display. If there are more than this many + * users, we'll show (maxVisible - 1) avatars and a "+N" indicator. + * + * In other words, if maxVisible is 5 and we have 5 users, we'll show all + * 5 avatars. If we have 6 users, we'll show 4 avatars and a "+2" label. + */ + maxVisible?: number; +} + +export const UserAvatarStack: FC = ({ users, maxVisible = 5, size = 32 }) => { + const { auth } = usePageProps(); + + const id = useId(); + + if (!users.length) { + return null; + } + + const userIntlLocale = getUserIntlLocale(auth?.user); + + const visibleCount = users.length > maxVisible ? maxVisible - 1 : maxVisible; + const visibleUsers = users.slice(0, visibleCount); + const remainingUsers = users.slice(visibleCount); + const remainingCount = remainingUsers.length; + + const formatter = new Intl.ListFormat(userIntlLocale, { style: 'long', type: 'conjunction' }); + const remainingNames = formatter.format(remainingUsers.map((user) => user.displayName).sort()); + + const numberFormatter = new Intl.NumberFormat(userIntlLocale, { signDisplay: 'always' }); + const formattedCount = numberFormatter.format(remainingCount); + + return ( +
+ {visibleUsers.map((user) => ( + + ))} + + {remainingCount > 0 ? ( + + +
+ {formattedCount} +
+ + + +

{remainingNames}

+ + + ) : null} +
+ ); +}; + +function getUserIntlLocale(user: AuthenticatedUser | undefined): string { + return user?.locale?.replace('_', '-') ?? 'en-us'; +} diff --git a/resources/js/common/components/UserAvatarStack/index.ts b/resources/js/common/components/UserAvatarStack/index.ts new file mode 100644 index 0000000000..e93d5db339 --- /dev/null +++ b/resources/js/common/components/UserAvatarStack/index.ts @@ -0,0 +1 @@ +export * from './UserAvatarStack'; diff --git a/resources/js/common/models/base-avatar-props.model.ts b/resources/js/common/models/base-avatar-props.model.ts index c5815e11db..af86f08d69 100644 --- a/resources/js/common/models/base-avatar-props.model.ts +++ b/resources/js/common/models/base-avatar-props.model.ts @@ -1,9 +1,13 @@ -// This is strongly typed so we don't wind up with 100 different possible sizes. -// If possible, use one of these sane defaults. Only add another one if necessary. -export type AvatarSize = 8 | 16 | 24 | 32 | 40 | 48 | 64 | 96 | 128; +/** + * This is strongly-typed so we don't wind up with 100 different possible sizes. + * If possible, use one of these sane defaults. + * When adding another one, try to ensure it corresponds to a Tailwind size-* class. + */ +export type AvatarSize = 8 | 16 | 24 | 28 | 32 | 40 | 48 | 64 | 96 | 128; export interface BaseAvatarProps { hasTooltip?: boolean; + imgClassName?: string; shouldLink?: boolean; showImage?: boolean; showLabel?: boolean; diff --git a/resources/js/features/game-list/components/AllGamesDataTable/useColumnDefinitions.ts b/resources/js/features/game-list/components/AllGamesDataTable/useColumnDefinitions.ts index b06e927db4..df8e24c3fa 100644 --- a/resources/js/features/game-list/components/AllGamesDataTable/useColumnDefinitions.ts +++ b/resources/js/features/game-list/components/AllGamesDataTable/useColumnDefinitions.ts @@ -48,7 +48,7 @@ export function useColumnDefinitions(options: { buildHasActiveOrInReviewClaimsColumnDef({ t_label: t('Claimed'), strings: { - t_description: t('One or more developers are currently working on this game.'), + t_no: t('No'), t_yes: t('Yes'), }, }), diff --git a/resources/js/features/game-list/components/AllSystemGamesDataTable/useColumnDefinitions.ts b/resources/js/features/game-list/components/AllSystemGamesDataTable/useColumnDefinitions.ts index b52030055d..c18180905d 100644 --- a/resources/js/features/game-list/components/AllSystemGamesDataTable/useColumnDefinitions.ts +++ b/resources/js/features/game-list/components/AllSystemGamesDataTable/useColumnDefinitions.ts @@ -91,7 +91,7 @@ export function useColumnDefinitions(options: { tableApiRouteParams, t_label: t('Claimed'), strings: { - t_description: t('One or more developers are currently working on this game.'), + t_no: t('No'), t_yes: t('Yes'), }, }), diff --git a/resources/js/features/game-list/components/GameListDataTable/GameListDataTable.test.tsx b/resources/js/features/game-list/components/GameListDataTable/GameListDataTable.test.tsx index 9619fc4117..879a8b5147 100644 --- a/resources/js/features/game-list/components/GameListDataTable/GameListDataTable.test.tsx +++ b/resources/js/features/game-list/components/GameListDataTable/GameListDataTable.test.tsx @@ -6,7 +6,12 @@ import type { FC } from 'react'; import i18n from '@/i18n-client'; import { render, screen } from '@/test'; -import { createGame, createGameListEntry, createSystem } from '@/test/factories'; +import { + createGame, + createGameClaimant, + createGameListEntry, + createSystem, +} from '@/test/factories'; import { buildHasActiveOrInReviewClaimsColumnDef } from '../../utils/column-definitions/buildHasActiveOrInReviewClaimsColumnDef'; import { buildLastUpdatedColumnDef } from '../../utils/column-definitions/buildLastUpdatedColumnDef'; @@ -355,6 +360,7 @@ describe('Component: GameListDataTable', () => { const game = createGame({ title: 'Sonic the Hedgehog', hasActiveOrInReviewClaims: undefined, // !! + claimants: [], }); const gameListEntry = createGameListEntry({ game }); @@ -365,8 +371,8 @@ describe('Component: GameListDataTable', () => { buildHasActiveOrInReviewClaimsColumnDef({ t_label: i18n.t('Claimed'), strings: { + t_no: i18n.t('No'), t_yes: i18n.t('Yes'), - t_description: i18n.t('One or more developers are currently working on this game.'), }, }), ]} @@ -377,7 +383,7 @@ describe('Component: GameListDataTable', () => { // ASSERT expect(screen.getByText(/sonic the hedgehog/i)).toBeVisible(); - expect(screen.getByText('-')).toBeVisible(); + expect(screen.getByText('No')).toBeVisible(); expect(screen.queryByText(/yes/i)).not.toBeInTheDocument(); }); @@ -386,6 +392,7 @@ describe('Component: GameListDataTable', () => { const game = createGame({ title: 'Sonic the Hedgehog', hasActiveOrInReviewClaims: true, // !! + claimants: [createGameClaimant()], }); const gameListEntry = createGameListEntry({ game }); @@ -396,8 +403,8 @@ describe('Component: GameListDataTable', () => { buildHasActiveOrInReviewClaimsColumnDef({ t_label: i18n.t('Claimed'), strings: { + t_no: i18n.t('No'), t_yes: i18n.t('Yes'), - t_description: i18n.t('One or more developers are currently working on this game.'), }, }), ]} @@ -408,8 +415,8 @@ describe('Component: GameListDataTable', () => { // ASSERT expect(screen.getByText(/sonic the hedgehog/i)).toBeVisible(); - expect(screen.queryByText('-')).not.toBeInTheDocument(); - expect(screen.getByText(/yes/i)).toBeVisible(); + expect(screen.queryByText('No')).not.toBeInTheDocument(); + expect(screen.getByText('Yes')).toBeVisible(); }); it('given there are 9 visible columns, applies the correct overflow scroll styles', () => { diff --git a/resources/js/features/game-list/components/HubGamesDataTable/useColumnDefinitions.ts b/resources/js/features/game-list/components/HubGamesDataTable/useColumnDefinitions.ts index b14c4ee970..a09ffd5895 100644 --- a/resources/js/features/game-list/components/HubGamesDataTable/useColumnDefinitions.ts +++ b/resources/js/features/game-list/components/HubGamesDataTable/useColumnDefinitions.ts @@ -92,10 +92,9 @@ export function useColumnDefinitions(options: { buildHasActiveOrInReviewClaimsColumnDef({ tableApiRouteName, tableApiRouteParams, - t_label: t('Claimed'), strings: { - t_description: t('One or more developers are currently working on this game.'), + t_no: t('No'), t_yes: t('Yes'), }, }), diff --git a/resources/js/features/game-list/components/WantToPlayGamesDataTable/useColumnDefinitions.ts b/resources/js/features/game-list/components/WantToPlayGamesDataTable/useColumnDefinitions.ts index 88376bdcdd..6af9137f76 100644 --- a/resources/js/features/game-list/components/WantToPlayGamesDataTable/useColumnDefinitions.ts +++ b/resources/js/features/game-list/components/WantToPlayGamesDataTable/useColumnDefinitions.ts @@ -63,7 +63,7 @@ export function useColumnDefinitions(options: { tableApiRouteName, t_label: t('Claimed'), strings: { - t_description: t('One or more developers are currently working on this game.'), + t_no: t('No'), t_yes: t('Yes'), }, }), diff --git a/resources/js/features/game-list/utils/column-definitions/buildHasActiveOrInReviewClaimsColumnDef.tsx b/resources/js/features/game-list/utils/column-definitions/buildHasActiveOrInReviewClaimsColumnDef.tsx index adc82ab103..28f10b184d 100644 --- a/resources/js/features/game-list/utils/column-definitions/buildHasActiveOrInReviewClaimsColumnDef.tsx +++ b/resources/js/features/game-list/utils/column-definitions/buildHasActiveOrInReviewClaimsColumnDef.tsx @@ -1,22 +1,18 @@ import type { ColumnDef } from '@tanstack/react-table'; import type { RouteName } from 'ziggy-js'; -import { - BaseTooltip, - BaseTooltipContent, - BaseTooltipTrigger, -} from '@/common/components/+vendor/BaseTooltip'; +import { UserAvatarStack } from '@/common/components/UserAvatarStack'; import type { TranslatedString } from '@/types/i18next'; import { DataTableColumnHeader } from '../../components/DataTableColumnHeader'; import { gameListFieldIconMap } from '../gameListFieldIconMap'; interface BuildHasActiveOrInReviewClaimsColumnDefProps { - t_label: TranslatedString; strings: { + t_no: TranslatedString; t_yes: TranslatedString; - t_description: TranslatedString; }; + t_label: TranslatedString; tableApiRouteName?: RouteName; tableApiRouteParams?: Record; @@ -47,18 +43,21 @@ export function buildHasActiveOrInReviewClaimsColumnDef({ return (
- {hasActiveOrInReviewClaims ? ( - - -

{strings.t_yes}

- + {hasActiveOrInReviewClaims && row.original.game.claimants ? ( +
+ {strings.t_yes} - -

{strings.t_description}

- - + c.user)} + maxVisible={3} + size={28} + /> +
) : ( -

{'-'}

+ <> + {'-'} + {strings.t_no} + )}
); diff --git a/resources/js/test/factories/createGameClaimant.ts b/resources/js/test/factories/createGameClaimant.ts new file mode 100644 index 0000000000..f3d46ee6fb --- /dev/null +++ b/resources/js/test/factories/createGameClaimant.ts @@ -0,0 +1,9 @@ +import { createFactory } from '../createFactory'; +import { createUser } from './createUser'; + +export const createGameClaimant = createFactory(() => { + return { + claimType: 'primary', + user: createUser(), + }; +}); diff --git a/resources/js/test/factories/index.ts b/resources/js/test/factories/index.ts index ed44b673bc..f779172ad7 100644 --- a/resources/js/test/factories/index.ts +++ b/resources/js/test/factories/index.ts @@ -6,6 +6,7 @@ export * from './createEmulator'; export * from './createEventAchievement'; export * from './createForumTopicComment'; export * from './createGame'; +export * from './createGameClaimant'; export * from './createGameHash'; export * from './createGameHashLabel'; export * from './createGameListEntry'; diff --git a/resources/js/types/generated.d.ts b/resources/js/types/generated.d.ts index bba0994f5a..3677defe7f 100644 --- a/resources/js/types/generated.d.ts +++ b/resources/js/types/generated.d.ts @@ -325,6 +325,10 @@ declare namespace App.Platform.Data { activeUntil?: string; forumTopicId?: number; }; + export type GameClaimant = { + user: App.Data.User; + claimType: string; + }; export type Game = { id: number; title: string; @@ -341,6 +345,7 @@ declare namespace App.Platform.Data { numVisibleLeaderboards?: number; numUnresolvedTickets?: number; hasActiveOrInReviewClaims?: boolean; + claimants?: Array; }; export type GameHash = { id: number;