Skip to content
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.

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

[MenuItem][Base] Pass idGenerator function #37364

Merged
merged 5 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 5 additions & 1 deletion packages/mui-base/src/useMenuItem/useMenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import {
import { useListItem } from '../useList';
import { useCompoundItem } from '../utils/useCompoundItem';

function idGenerator(existingKeys: Set<string>) {
Copy link
Contributor Author

@sai6855 sai6855 May 23, 2023

Choose a reason for hiding this comment

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

Not really sure how to test that if this function is being called or not as this function runs only on react versions less than 18.

return `menu-item-${existingKeys.size}`;
}

/**
*
* Demos:
Expand Down Expand Up @@ -40,7 +44,7 @@ export default function useMenuItem(params: UseMenuItemParameters): UseMenuItemR
item: id,
});

const { index, totalItemCount } = useCompoundItem(id, itemMetadata);
const { index, totalItemCount } = useCompoundItem(id ?? idGenerator, itemMetadata);

const {
getRootProps: getButtonProps,
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/useTab/useTab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function useTab(parameters: UseTabParameters): UseTabReturnValue {
id: value,
index,
totalItemCount: totalTabsCount,
} = useCompoundItem<string | number, TabMetadata>(valueParam, tabMetadata, tabValueGenerator);
} = useCompoundItem<string | number, TabMetadata>(valueParam ?? tabValueGenerator, tabMetadata);

const {
getRootProps: getTabProps,
Expand Down
2 changes: 1 addition & 1 deletion packages/mui-base/src/useTabPanel/useTabPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function useTabPanel(parameters: UseTabPanelParameters): UseTabPanelReturnValue
const handleRef = useForkRef(ref, externalRef);
const metadata = React.useMemo(() => ({ id, ref }), [id]);

const { id: value } = useCompoundItem(valueParam, metadata, tabPanelValueGenerator);
const { id: value } = useCompoundItem(valueParam ?? tabPanelValueGenerator, metadata);

const hidden = value !== selectedTabValue;

Expand Down
3 changes: 1 addition & 2 deletions packages/mui-base/src/utils/useCompound.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -257,9 +257,8 @@ describe('compound components', () => {
function Child() {
const ref = React.useRef<HTMLLIElement>(null);
const { id } = useCompoundItem<string, { ref: React.RefObject<HTMLLIElement> }>(
undefined,
React.useMemo(() => ({ ref }), []),
idGenerator,
React.useMemo(() => ({ ref }), []),
);

return <li ref={ref}>{id}</li>;
Expand Down
32 changes: 10 additions & 22 deletions packages/mui-base/src/utils/useCompound.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,20 @@ interface RegisterItemReturnValue<Key> {
deregister: () => void;
}

export type KeyGenerator<Key> = (existingKeys: Set<Key>) => Key;

export type CompoundComponentContextValue<Key, Subitem> = {
/**
* Registers an item with the parent.
* This should be called during the effect phase of the child component.
* The `itemMetadata` should be a stable reference (e.g. a memoized object), to avoid unnecessary re-registrations.
*
* @param id Id of the item.
* @param itemMetadata Arbitrary metadata to pass to the parent component.
* @param missingKeyGenerator A function that generates a unique id for the item.
* @param id Id of the item or A function that generates a unique id for the item.
* It is called with the set of the ids of all the items that have already been registered.
* Return `existingKeys.size` if you want to use the index of the new item as the id.
* Return `existingKeys.size` if you want to use the index of the new item as the id..
* @param itemMetadata Arbitrary metadata to pass to the parent component.
*/
registerItem: (
id: Key | undefined,
item: Subitem,
missingKeyGenerator?: (existingKeys: Set<Key>) => Key,
) => RegisterItemReturnValue<Key>;
registerItem: (id: Key | KeyGenerator<Key>, item: Subitem) => RegisterItemReturnValue<Key>;
/**
* Returns the 0-based index of the item in the parent component's list of registered items.
*
Expand Down Expand Up @@ -116,20 +113,11 @@ export function useCompoundParent<
}, []);

const registerItem = React.useCallback(
function registerItem(
id: Key | undefined,
item: Subitem,
missingKeyGenerator?: (existingKeys: Set<Key>) => Key,
) {
function registerItem(id: Key | KeyGenerator<Key>, item: Subitem) {
let providedOrGeneratedId: Key;
if (id === undefined) {
if (missingKeyGenerator === undefined) {
throw new Error(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since id cannot be undefined, i don't think throwing error makes much sense

"The compound component's child doesn't have a key. You need to provide a missingKeyGenerator to generate it.",
);
}

providedOrGeneratedId = missingKeyGenerator(subitemKeys.current);

if (typeof id === 'function') {
providedOrGeneratedId = (id as KeyGenerator<Key>)(subitemKeys.current);
} else {
providedOrGeneratedId = id;
}
Expand Down
20 changes: 11 additions & 9 deletions packages/mui-base/src/utils/useCompoundItem.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import * as React from 'react';
import { unstable_useEnhancedEffect as useEnhancedEffect } from '@mui/utils';
import { CompoundComponentContext, CompoundComponentContextValue } from './useCompound';
import {
CompoundComponentContext,
CompoundComponentContextValue,
KeyGenerator,
} from './useCompound';

export interface UseCompoundItemReturnValue<Key> {
/**
Expand Down Expand Up @@ -31,19 +35,17 @@ export interface UseCompoundItemReturnValue<Key> {
*
* @ignore - internal hook.
*/
export function useCompoundItem<Key, Subitem extends { ref: React.RefObject<Node> }>(
id: Key | undefined,
export function useCompoundItem<Key, Subitem>(
id: Key | KeyGenerator<Key>,
itemMetadata: Subitem,
missingKeyGenerator: (existingKeys: Set<Key>) => Key,
): UseCompoundItemReturnValue<Key>;
export function useCompoundItem<Key, Subitem>(
id: Key,
itemMetadata: Subitem,
): UseCompoundItemReturnValue<Key>;
export function useCompoundItem<Key, Subitem>(
id: Key | undefined,
id: Key | KeyGenerator<Key>,
itemMetadata: Subitem,
missingKeyGenerator?: (existingKeys: Set<Key>) => Key,
): UseCompoundItemReturnValue<Key> {
const context = React.useContext(CompoundComponentContext) as CompoundComponentContextValue<
Key,
Expand All @@ -55,13 +57,13 @@ export function useCompoundItem<Key, Subitem>(
}

const { registerItem } = context;
const [registeredId, setRegisteredId] = React.useState(id);
const [registeredId, setRegisteredId] = React.useState(typeof id === 'function' ? undefined : id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since parameter to pass to the id function is not available at this point, i initiated value of registeredId to undefined if id is function.


useEnhancedEffect(() => {
const { id: returnedId, deregister } = registerItem(id, itemMetadata, missingKeyGenerator);
const { id: returnedId, deregister } = registerItem(id, itemMetadata);
setRegisteredId(returnedId);
return deregister;
}, [registerItem, itemMetadata, missingKeyGenerator, id]);
}, [registerItem, itemMetadata, id]);

return {
id: registeredId,
Expand Down