Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: consume shared AvatarContext",
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) Update this description now that AvatarContext is implemented in react-avatar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in ef70cc8

"packageName": "@fluentui/react-avatar",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: Implement AvatarContext",
"packageName": "@fluentui/react-shared-contexts",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in ef70cc8

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "feat: Use AvatarContext to override avatar size",
"packageName": "@fluentui/react-table",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,19 @@ import { getInitials } from '../../utils/index';
import type { AvatarNamedColor, AvatarProps, AvatarState } from './Avatar.types';
import { PersonRegular } from '@fluentui/react-icons';
import { PresenceBadge } from '@fluentui/react-badge';
import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
import { useFluent_unstable as useFluent, useAvatarContext } from '@fluentui/react-shared-contexts';

export const useAvatar_unstable = (props: AvatarProps, ref: React.Ref<HTMLElement>): AvatarState => {
const { dir } = useFluent();
const { name, size = 32, shape = 'circular', active = 'unset', activeAppearance = 'ring', idForColor } = props;
const { size: contextSize } = useAvatarContext();
const {
name,
size = contextSize ?? (32 as const),
shape = 'circular',
active = 'unset',
activeAppearance = 'ring',
idForColor,
} = props;
let { color = 'neutral' } = props;

// Resolve 'colorful' to a specific color name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@
import * as React_2 from 'react';
import type { Theme } from '@fluentui/react-theme';

// @internal (undocumented)
export const AvatarContextProvider: React_2.Provider<AvatarContextValue | undefined>;

// @internal (undocumented)
export interface AvatarContextValue {
// (undocumented)
size?: AvatarSizes;
}

// @internal (undocumented)
export const Provider_unstable: React_2.Provider<ProviderContextValue_unstable>;

Expand Down Expand Up @@ -41,6 +50,9 @@ export type TooltipVisibilityContextValue_unstable = {
// @internal (undocumented)
export const TooltipVisibilityProvider_unstable: React_2.Provider<TooltipVisibilityContextValue_unstable>;

// @internal (undocumented)
export const useAvatarContext: () => AvatarContextValue;

// @public (undocumented)
export function useFluent_unstable(): ProviderContextValue_unstable;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

It's up to you, but I think that it should be hosted in a separate package as portal-compat-context.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or is there any real issue with including it in the react-avatar package? The rest of react-avatar should be tree shaken out of react-table if it's not used, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't mind, I've moved the avatar context to react-avatar in e29a351


const avatarContext = React.createContext<AvatarContextValue | undefined>(undefined);

/**
* Sizes that can be used for the Avatar
* Copied from the react-avatar package, if the sizes are incompatible then TS should fail
*/
type AvatarSizes = 16 | 20 | 24 | 28 | 32 | 36 | 40 | 48 | 56 | 64 | 72 | 96 | 120 | 128;

/**
* @internal
*/
export interface AvatarContextValue {
size?: AvatarSizes;
}

const avatarContextDefaultValue: AvatarContextValue = {};

/**
* @internal
*/
export const AvatarContextProvider = avatarContext.Provider;

/**
* @internal
*/
export const useAvatarContext = () => React.useContext(avatarContext) ?? avatarContextDefaultValue;
3 changes: 3 additions & 0 deletions packages/react-components/react-shared-contexts/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ export type { TooltipVisibilityContextValue as TooltipVisibilityContextValue_uns

export { Provider as Provider_unstable, useFluent as useFluent_unstable } from './ProviderContext';
export type { ProviderContextValue as ProviderContextValue_unstable } from './ProviderContext';

export { AvatarContextProvider, useAvatarContext } from './AvatarContext';
export type { AvatarContextValue } from './AvatarContext';
1 change: 1 addition & 0 deletions packages/react-components/react-table/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@fluentui/react-aria": "^9.1.0",
"@fluentui/react-checkbox": "^9.0.4",
"@fluentui/react-icons": "^2.0.175",
"@fluentui/react-shared-contexts": "^9.0.0",
"@fluentui/react-tabster": "^9.1.0",
"@fluentui/react-theme": "^9.0.0",
"@fluentui/react-utilities": "^9.0.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { useTableCellLayout_unstable } from './useTableCellLayout';
import { renderTableCellLayout_unstable } from './renderTableCellLayout';
import { useTableCellLayoutStyles_unstable } from './useTableCellLayoutStyles';
import { useTableCellLayoutContextValues_unstable } from './useTableCellLayoutContextValues';
import type { TableCellLayoutProps } from './TableCellLayout.types';
import type { ForwardRefComponent } from '@fluentui/react-utilities';

Expand All @@ -12,7 +13,7 @@ export const TableCellLayout: ForwardRefComponent<TableCellLayoutProps> = React.
const state = useTableCellLayout_unstable(props, ref);

useTableCellLayoutStyles_unstable(state);
return renderTableCellLayout_unstable(state);
return renderTableCellLayout_unstable(state, useTableCellLayoutContextValues_unstable(state));
});

TableCellLayout.displayName = 'TableCellLayout';
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities';
import type { AvatarContextValue } from '@fluentui/react-shared-contexts';
import type { TableContextValue } from '../Table/Table.types';

export type TableCellLayoutContextValues = {
avatar: AvatarContextValue;
};

export type TableCellLayoutSlots = {
root: Slot<'div'>;
Expand Down Expand Up @@ -34,4 +40,5 @@ export type TableCellLayoutProps = ComponentProps<Partial<TableCellLayoutSlots>>
/**
* State used in rendering TableCellLayout
*/
export type TableCellLayoutState = ComponentState<TableCellLayoutSlots> & Pick<TableCellLayoutProps, 'appearance'>;
export type TableCellLayoutState = ComponentState<TableCellLayoutSlots> &
Pick<TableCellLayoutProps, 'appearance'> & { size: TableContextValue['size'] };
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import * as React from 'react';
import { getSlots } from '@fluentui/react-utilities';
import type { TableCellLayoutState, TableCellLayoutSlots } from './TableCellLayout.types';
import { AvatarContextProvider } from '@fluentui/react-shared-contexts';
import type { TableCellLayoutState, TableCellLayoutSlots, TableCellLayoutContextValues } from './TableCellLayout.types';

/**
* Render the final JSX of TableCellLayout
*/
export const renderTableCellLayout_unstable = (state: TableCellLayoutState) => {
export const renderTableCellLayout_unstable = (
state: TableCellLayoutState,
contextValues: TableCellLayoutContextValues,
) => {
const { slots, slotProps } = getSlots<TableCellLayoutSlots>(state);

return (
<slots.root {...slotProps.root}>
{slots.media && <slots.media {...slotProps.media} />}
<AvatarContextProvider value={contextValues.avatar}>
{slots.media && <slots.media {...slotProps.media} />}
</AvatarContextProvider>
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need to include the AvatarContextProvider if there's no media content:

Suggested change
<AvatarContextProvider value={contextValues.avatar}>
{slots.media && <slots.media {...slotProps.media} />}
</AvatarContextProvider>
{slots.media && (
<AvatarContextProvider value={contextValues.avatar}>
<slots.media {...slotProps.media} />
</AvatarContextProvider>
)}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e29a351

{slots.wrapper && (
<slots.wrapper {...slotProps.wrapper}>
{slots.main && <slots.main {...slotProps.main}>{slotProps.root.children}</slots.main>}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import { getNativeElementProps, resolveShorthand } from '@fluentui/react-utilities';
import type { TableCellLayoutProps, TableCellLayoutState } from './TableCellLayout.types';
import { useTableContext } from '../../contexts/tableContext';

/**
* Create the state required to render TableCellLayout.
Expand All @@ -15,6 +16,8 @@ export const useTableCellLayout_unstable = (
props: TableCellLayoutProps,
ref: React.Ref<HTMLElement>,
): TableCellLayoutState => {
const { size } = useTableContext();

return {
components: {
root: 'div',
Expand All @@ -29,5 +32,6 @@ export const useTableCellLayout_unstable = (
media: resolveShorthand(props.media),
description: resolveShorthand(props.description),
wrapper: resolveShorthand(props.wrapper, { required: !!props.description || !!props.children }),
size,
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as React from 'react';
import type { TableCellLayoutState, TableCellLayoutContextValues } from './TableCellLayout.types';

const tableAvatarSizeMap = {
medium: undefined,
small: 24,
smaller: 20,
} as const;

export function useTableCellLayoutContextValues_unstable(state: TableCellLayoutState): TableCellLayoutContextValues {
const { size: tableSize } = state;

const avatar = React.useMemo(
() => ({
size: tableAvatarSizeMap[tableSize],
Copy link
Member

Choose a reason for hiding this comment

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

IMO, a value should be stored in state so it could be overridden with composition patterns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right, I added avatarSize to state

}),
[tableSize],
);

return {
avatar,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,7 @@ export const SizeSmall = () => {
<TableCell>
<TableCellLayout
media={
<Avatar
name={item.author.label}
badge={{ status: item.author.status as PresenceBadgeStatus }}
size={24}
/>
<Avatar name={item.author.label} badge={{ status: item.author.status as PresenceBadgeStatus }} />
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) This is unrelated to this change, but we should encourage as casts like this in stories. You could either define the entire items array as const, or possibly adding making the individual statuses as const in the object, like status: 'available' as const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in bcf7dab

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! I just realized I left out "not" in "should not encourage" 🤦‍♂️ but I think you figured it out 🙂

}
>
{item.author.label}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,7 @@ export const SizeSmaller = () => {
<TableCell>
<TableCellLayout
media={
<Avatar
name={item.author.label}
badge={{ status: item.author.status as PresenceBadgeStatus }}
size={20}
/>
<Avatar name={item.author.label} badge={{ status: item.author.status as PresenceBadgeStatus }} />
}
>
{item.author.label}
Expand Down