From 5e7f077eeb08deb6bdaca45a25f6baec782dccd6 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Tue, 26 Nov 2024 12:39:42 -0300 Subject: [PATCH 1/3] fix: `UserAvatar` should always render `BaseAvatar` --- .../ui-avatar/src/components/UserAvatar.tsx | 20 +++++-------------- .../src/hooks/useUserAvatarPath.ts | 4 ++-- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/packages/ui-avatar/src/components/UserAvatar.tsx b/packages/ui-avatar/src/components/UserAvatar.tsx index 9945aa2b26b0..4491a8ba0548 100644 --- a/packages/ui-avatar/src/components/UserAvatar.tsx +++ b/packages/ui-avatar/src/components/UserAvatar.tsx @@ -4,20 +4,13 @@ import { memo } from 'react'; import type { BaseAvatarProps } from './BaseAvatar'; import BaseAvatar from './BaseAvatar'; -type UsernameProp = { - username: string; - userId?: never; -}; - -type UserIdProp = { - userId: string; - username?: never; -}; type UserAvatarProps = Omit & { + username: string; + userId?: string; etag?: string; url?: string; title?: string; -} & (UsernameProp | UserIdProp); +}; const UserAvatar = ({ username, userId, etag, ...rest }: UserAvatarProps) => { const getUserAvatarPath = useUserAvatarPath(); @@ -26,12 +19,9 @@ const UserAvatar = ({ username, userId, etag, ...rest }: UserAvatarProps) => { const { url = getUserAvatarPath({ userId, etag }), ...props } = rest; return ; } - if (username) { - const { url = getUserAvatarPath({ username, etag }), ...props } = rest; - return ; - } - throw new Error('ui-avatar(UserAvatar) - Either username or userId must be provided'); + const { url = getUserAvatarPath({ username, etag }), ...props } = rest; + return ; }; export default memo(UserAvatar); diff --git a/packages/ui-contexts/src/hooks/useUserAvatarPath.ts b/packages/ui-contexts/src/hooks/useUserAvatarPath.ts index 5411bf089ee3..756f64c56591 100644 --- a/packages/ui-contexts/src/hooks/useUserAvatarPath.ts +++ b/packages/ui-contexts/src/hooks/useUserAvatarPath.ts @@ -1,5 +1,5 @@ import { useContext } from 'react'; -import { AvatarUrlContext, type AvatarUrlContextValue } from '../AvatarUrlContext'; +import { AvatarUrlContext } from '../AvatarUrlContext'; -export const useUserAvatarPath = (): AvatarUrlContextValue['getUserPathAvatar'] => useContext(AvatarUrlContext).getUserPathAvatar; +export const useUserAvatarPath = () => useContext(AvatarUrlContext).getUserPathAvatar; From 41445d2d307bb2a670626abffe67b40834c4a116 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Tue, 26 Nov 2024 17:19:19 -0300 Subject: [PATCH 2/3] chore: ts --- apps/meteor/client/providers/AvatarUrlProvider.tsx | 2 +- packages/ui-avatar/src/components/UserAvatar.tsx | 14 +++++++++++--- packages/ui-contexts/src/AvatarUrlContext.ts | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/apps/meteor/client/providers/AvatarUrlProvider.tsx b/apps/meteor/client/providers/AvatarUrlProvider.tsx index 056bc10e0d52..ee23ca071ee4 100644 --- a/apps/meteor/client/providers/AvatarUrlProvider.tsx +++ b/apps/meteor/client/providers/AvatarUrlProvider.tsx @@ -13,7 +13,7 @@ const AvatarUrlProvider = ({ children }: AvatarUrlProviderProps) => { const contextValue = useMemo(() => { function getUserPathAvatar(username: string, etag?: string): string; function getUserPathAvatar({ userId, etag }: { userId: string; etag?: string }): string; - function getUserPathAvatar({ username, etag }: { username: string; etag?: string }): string; + function getUserPathAvatar({ username, etag }: { username?: string; etag?: string }): string; function getUserPathAvatar(...args: any): string { if (typeof args[0] === 'string') { const [username, etag] = args; diff --git a/packages/ui-avatar/src/components/UserAvatar.tsx b/packages/ui-avatar/src/components/UserAvatar.tsx index 4491a8ba0548..ce15ff8fca09 100644 --- a/packages/ui-avatar/src/components/UserAvatar.tsx +++ b/packages/ui-avatar/src/components/UserAvatar.tsx @@ -4,13 +4,21 @@ import { memo } from 'react'; import type { BaseAvatarProps } from './BaseAvatar'; import BaseAvatar from './BaseAvatar'; -type UserAvatarProps = Omit & { +type UsernameProp = { username: string; - userId?: string; + userId?: never; +}; + +type UserIdProp = { + userId: string; + username?: never; +}; + +type UserAvatarProps = Omit & { etag?: string; url?: string; title?: string; -}; +} & (UsernameProp | UserIdProp); const UserAvatar = ({ username, userId, etag, ...rest }: UserAvatarProps) => { const getUserAvatarPath = useUserAvatarPath(); diff --git a/packages/ui-contexts/src/AvatarUrlContext.ts b/packages/ui-contexts/src/AvatarUrlContext.ts index 1f13e2764720..760c89bc2dc0 100644 --- a/packages/ui-contexts/src/AvatarUrlContext.ts +++ b/packages/ui-contexts/src/AvatarUrlContext.ts @@ -6,7 +6,7 @@ export type AvatarUrlContextValue = { getUserPathAvatar: { (username: string, etag?: string): string; (params: { userId: string; etag?: string }): string; - (params: { username: string; etag?: string }): string; + (params: { username?: string; etag?: string }): string; }; getRoomPathAvatar: (...args: any) => string; }; From d99cf5c0bda48c92d1327c4e3baa309992cbf8ea Mon Sep 17 00:00:00 2001 From: dougfabris Date: Thu, 28 Nov 2024 12:47:45 -0300 Subject: [PATCH 3/3] fix: review --- apps/meteor/client/providers/AvatarUrlProvider.tsx | 2 +- packages/ui-avatar/src/components/UserAvatar.tsx | 9 ++++++--- packages/ui-contexts/src/AvatarUrlContext.ts | 2 +- packages/ui-contexts/src/hooks/useUserAvatarPath.ts | 4 ++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/apps/meteor/client/providers/AvatarUrlProvider.tsx b/apps/meteor/client/providers/AvatarUrlProvider.tsx index ee23ca071ee4..056bc10e0d52 100644 --- a/apps/meteor/client/providers/AvatarUrlProvider.tsx +++ b/apps/meteor/client/providers/AvatarUrlProvider.tsx @@ -13,7 +13,7 @@ const AvatarUrlProvider = ({ children }: AvatarUrlProviderProps) => { const contextValue = useMemo(() => { function getUserPathAvatar(username: string, etag?: string): string; function getUserPathAvatar({ userId, etag }: { userId: string; etag?: string }): string; - function getUserPathAvatar({ username, etag }: { username?: string; etag?: string }): string; + function getUserPathAvatar({ username, etag }: { username: string; etag?: string }): string; function getUserPathAvatar(...args: any): string { if (typeof args[0] === 'string') { const [username, etag] = args; diff --git a/packages/ui-avatar/src/components/UserAvatar.tsx b/packages/ui-avatar/src/components/UserAvatar.tsx index ce15ff8fca09..b4f5ae85a658 100644 --- a/packages/ui-avatar/src/components/UserAvatar.tsx +++ b/packages/ui-avatar/src/components/UserAvatar.tsx @@ -13,7 +13,6 @@ type UserIdProp = { userId: string; username?: never; }; - type UserAvatarProps = Omit & { etag?: string; url?: string; @@ -27,9 +26,13 @@ const UserAvatar = ({ username, userId, etag, ...rest }: UserAvatarProps) => { const { url = getUserAvatarPath({ userId, etag }), ...props } = rest; return ; } + if (username) { + const { url = getUserAvatarPath({ username, etag }), ...props } = rest; + return ; + } - const { url = getUserAvatarPath({ username, etag }), ...props } = rest; - return ; + // TODO: We should throw an Error after fixing the issue in Composer passing the username undefined + return null; }; export default memo(UserAvatar); diff --git a/packages/ui-contexts/src/AvatarUrlContext.ts b/packages/ui-contexts/src/AvatarUrlContext.ts index 760c89bc2dc0..1f13e2764720 100644 --- a/packages/ui-contexts/src/AvatarUrlContext.ts +++ b/packages/ui-contexts/src/AvatarUrlContext.ts @@ -6,7 +6,7 @@ export type AvatarUrlContextValue = { getUserPathAvatar: { (username: string, etag?: string): string; (params: { userId: string; etag?: string }): string; - (params: { username?: string; etag?: string }): string; + (params: { username: string; etag?: string }): string; }; getRoomPathAvatar: (...args: any) => string; }; diff --git a/packages/ui-contexts/src/hooks/useUserAvatarPath.ts b/packages/ui-contexts/src/hooks/useUserAvatarPath.ts index 756f64c56591..5411bf089ee3 100644 --- a/packages/ui-contexts/src/hooks/useUserAvatarPath.ts +++ b/packages/ui-contexts/src/hooks/useUserAvatarPath.ts @@ -1,5 +1,5 @@ import { useContext } from 'react'; -import { AvatarUrlContext } from '../AvatarUrlContext'; +import { AvatarUrlContext, type AvatarUrlContextValue } from '../AvatarUrlContext'; -export const useUserAvatarPath = () => useContext(AvatarUrlContext).getUserPathAvatar; +export const useUserAvatarPath = (): AvatarUrlContextValue['getUserPathAvatar'] => useContext(AvatarUrlContext).getUserPathAvatar;