Skip to content

Commit

Permalink
feat(Menu): remove parent context flags with new hook (#18812)
Browse files Browse the repository at this point in the history
* feat(Menu): remove parent context flags with new hook

Removes the `hasMenuContext` and `hasMenuListContext` flags in `Menu`
contexts and replaces them with a new `useHasParentContext` hook in
`react-context-selectors`

* Change files

* fix docs

* revert web-components

* update README
  • Loading branch information
ling1726 committed Jul 14, 2021
1 parent 6dbf1ed commit fce57b2
Show file tree
Hide file tree
Showing 16 changed files with 112 additions and 27 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "feat(useHasParentContext): new hook to determine if a context provider exists as parent",
"packageName": "@fluentui/react-context-selector",
"email": "lingfan.gao@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "feat(Menu): remove parent context flags with new hook",
"packageName": "@fluentui/react-menu",
"email": "lingfan.gao@microsoft.com",
"dependentChangeType": "patch"
}
20 changes: 20 additions & 0 deletions packages/react-context-selector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ yarn add @fluentui/react-context-selector

## Usage

### Getting started

```tsx
import * as React from 'react';
import { createContext, useContextSelector, ContextSelector } from '@fluentui/react-context-selector';
Expand Down Expand Up @@ -86,6 +88,24 @@ export default function App() {
}
```

### useHasParentContext

This helper hook will allow you to know if a component is wrapped by a context selector provider

```tsx
const Foo = () => {
// An easy way to test if a context provider is wrapped around this component
// since it's more complicated to compare with a default context value
const isWrappedWithContext = useHasParentContext(CounterContext);

if (isWrappedWithContext) {
return <div>I am inside context selector provider</div>;
} else {
return <div>I can only use default context value</div>;
}
};
```

## Technical memo

React context by nature triggers propagation of component re-rendering if a value is changed. To avoid this, this library uses undocumented feature of `calculateChangedBits`. It then uses a subscription model to force update when a component needs to re-render.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ export const createContext: <Value>(defaultValue: Value) => Context<Value>;
// @public
export const useContextSelector: <Value, SelectedValue>(context: Context<Value>, selector: ContextSelector<Value, SelectedValue>) => SelectedValue;

// @public
export function useHasParentContext<Value>(context: Context<Value>): boolean;


// (No @packageDocumentation comment for this package)

Expand Down
1 change: 1 addition & 0 deletions packages/react-context-selector/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { createContext } from './createContext';
export { useContextSelector } from './useContextSelector';
export { useHasParentContext } from './useHasParentContext';
export * from './types';
25 changes: 25 additions & 0 deletions packages/react-context-selector/src/useHasParentContext.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as React from 'react';
import { renderHook } from '@testing-library/react-hooks';
import { createContext } from './createContext';
import { useHasParentContext } from './useHasParentContext';

const TestContext = createContext<number>(1);

describe('useHasParentContext', () => {
it('should return true if wrapped with context', () => {
// Arrange
const wrapper: React.FC = ({ children }) => <TestContext.Provider value={1}>{children}</TestContext.Provider>;
const { result } = renderHook(() => useHasParentContext(TestContext), { wrapper });

// Assert
expect(result.current).toBe(true);
});

it('should return true if not wrapped with context', () => {
// Arrange
const { result } = renderHook(() => useHasParentContext(TestContext));

// Assert
expect(result.current).toBe(false);
});
});
19 changes: 19 additions & 0 deletions packages/react-context-selector/src/useHasParentContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as React from 'react';
import { Context, ContextValue } from './types';

/**
* Utility hook for contexts created by react-context-selector to determine if a parent context exists
* WARNING: This hook will not work for native React contexts
*
* @param context - context created by react-context-selector
* @returns whether the hook is wrapped by a parent context
*/
export function useHasParentContext<Value>(context: Context<Value>) {
const contextValue = React.useContext((context as unknown) as Context<ContextValue<Value>>);

if (contextValue.version) {
return contextValue.version.current !== -1;
}

return false;
}
11 changes: 7 additions & 4 deletions packages/react-menu/etc/react-menu.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { ComponentProps } from '@fluentui/react-utilities';
import { ComponentState } from '@fluentui/react-utilities';
import { Context } from '@fluentui/react-context-selector';
import { ContextSelector } from '@fluentui/react-context-selector';
import { ObjectShorthandProps } from '@fluentui/react-utilities';
import { PositioningProps } from '@fluentui/react-positioning';
Expand All @@ -15,10 +16,11 @@ import { usePopperMouseTarget } from '@fluentui/react-positioning';
// @public
export const Menu: React_2.FC<MenuProps>;

// @public (undocumented)
export const MenuContext: Context<MenuContextValue>;

// @public
export interface MenuContextValue extends MenuListProps, Pick<MenuState, 'openOnHover' | 'openOnContext' | 'triggerRef' | 'menuPopoverRef' | 'setOpen' | 'isSubmenu' | 'triggerId' | 'hasIcons' | 'hasCheckmarks' | 'persistOnItemClick' | 'inline'> {
// (undocumented)
hasMenuContext: boolean;
// (undocumented)
open: boolean;
// (undocumented)
Expand Down Expand Up @@ -137,10 +139,11 @@ export interface MenuItemState extends ComponentState<MenuItemSlots>, React_2.HT
// @public
export const MenuList: React_2.FunctionComponent<MenuListProps & React_2.RefAttributes<HTMLElement>>;

// @public (undocumented)
export const MenuListContext: Context<MenuListContextValue>;

// @public
export interface MenuListContextValue extends Pick<MenuListProps, 'checkedValues' | 'onCheckedValueChange' | 'hasIcons' | 'hasCheckmarks'> {
// (undocumented)
hasMenuListContext?: boolean;
// (undocumented)
selectRadio?: SelectableHandler;
// (undocumented)
Expand Down
1 change: 0 additions & 1 deletion packages/react-menu/src/common/mockUseMenuContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export const mockUseMenuContext = (options: Partial<MenuContextValue> = {}) => {
menuPopoverRef: ({ current: null } as unknown) as React.MutableRefObject<HTMLElement>,
openOnContext: false,
openOnHover: false,
hasMenuContext: false,
isSubmenu: false,
triggerId: 'id',
...options,
Expand Down
1 change: 0 additions & 1 deletion packages/react-menu/src/components/Menu/renderMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export const renderMenu = (state: MenuState) => {
triggerId,
menuPopoverRef,
isSubmenu,
hasMenuContext: true,
hasCheckmarks,
hasIcons,
persistOnItemClick,
Expand Down
5 changes: 3 additions & 2 deletions packages/react-menu/src/components/MenuList/MenuList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import { MenuList } from './MenuList';
import * as renderer from 'react-test-renderer';
import { ReactWrapper } from 'enzyme';
import { render } from '@testing-library/react';
import { useHasParentContext } from '@fluentui/react-context-selector';
import { isConformant } from '../../common/isConformant';
import { MenuListProvider, useMenuListContext } from '../../contexts/menuListContext';
import { MenuListContext, MenuListProvider } from '../../contexts/menuListContext';

describe('MenuList', () => {
isConformant({
Expand Down Expand Up @@ -35,7 +36,7 @@ describe('MenuList', () => {
// Arrange
let hasMenuListContext: boolean | undefined = false;
const TestComponent = () => {
hasMenuListContext = useMenuListContext(context => context.hasMenuListContext);
hasMenuListContext = useHasParentContext(MenuListContext);
return null;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export const renderMenuList = (state: MenuListState) => {
setFocusByFirstCharacter,
hasIcons,
hasCheckmarks,
hasMenuListContext: true,
}}
>
<slots.root {...slotProps.root}>{state.children}</slots.root>
Expand Down
19 changes: 12 additions & 7 deletions packages/react-menu/src/components/MenuList/useMenuList.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import * as React from 'react';
import { useMergedRefs, useEventCallback, useControllableValue } from '@fluentui/react-utilities';
import { useArrowNavigationGroup, useFocusFinders } from '@fluentui/react-tabster';
import { useHasParentContext } from '@fluentui/react-context-selector';
import { MenuListProps, MenuListState, UninitializedMenuListState } from './MenuList.types';
import { useMenuContext } from '../../contexts/menuContext';
import { MenuContext } from '../../contexts/menuContext';

/**
* Returns the props and state required to render the component
Expand All @@ -11,8 +13,9 @@ export const useMenuList = (props: MenuListProps, ref: React.Ref<HTMLElement>):
const focusAttributes = useArrowNavigationGroup({ circular: true });
const { findAllFocusable } = useFocusFinders();
const menuContext = useMenuContextSelectors();
const hasMenuContext = useHasParentContext(MenuContext);

if (usingPropsAndMenuContext(props, menuContext)) {
if (usingPropsAndMenuContext(props, menuContext, hasMenuContext)) {
// TODO throw warnings in development safely
// eslint-disable-next-line no-console
console.warn('You are using both MenuList and Menu props, we recommend you to use Menu props when available');
Expand All @@ -26,10 +29,10 @@ export const useMenuList = (props: MenuListProps, ref: React.Ref<HTMLElement>):
hasIcons: menuContext.hasIcons,
hasCheckmarks: menuContext.hasCheckmarks,
...focusAttributes,
...(menuContext.hasMenuContext && menuContext),
...(hasMenuContext && menuContext),
// TODO: This is needed because Menu 'controls' the MenuList and will cause switching controlled state warnings
// Solution is to define 'initial value' in useControllableValue like in v0
...(menuContext.hasMenuContext && !menuContext.checkedValues && { checkedValues: {} }),
...(hasMenuContext && !menuContext.checkedValues && { checkedValues: {} }),
...props,
};

Expand Down Expand Up @@ -121,7 +124,6 @@ export const useMenuList = (props: MenuListProps, ref: React.Ref<HTMLElement>):
* Adds some sugar to fetching multiple context selector values
*/
const useMenuContextSelectors = () => {
const hasMenuContext = useMenuContext(context => context.hasMenuContext);
const checkedValues = useMenuContext(context => context.checkedValues);
const onCheckedValueChange = useMenuContext(context => context.onCheckedValueChange);
const defaultCheckedValues = useMenuContext(context => context.defaultCheckedValues);
Expand All @@ -130,7 +132,6 @@ const useMenuContextSelectors = () => {
const hasCheckmarks = useMenuContext(context => context.hasCheckmarks);

return {
hasMenuContext,
checkedValues,
onCheckedValueChange,
defaultCheckedValues,
Expand All @@ -143,13 +144,17 @@ const useMenuContextSelectors = () => {
/**
* Helper function to detect if props and MenuContext values are both used
*/
const usingPropsAndMenuContext = (props: MenuListProps, contextValue: ReturnType<typeof useMenuContextSelectors>) => {
const usingPropsAndMenuContext = (
props: MenuListProps,
contextValue: ReturnType<typeof useMenuContextSelectors>,
hasMenuContext: boolean,
) => {
let isUsingPropsAndContext = false;
for (const val in contextValue) {
if (props[val as keyof Omit<typeof contextValue, 'hasMenuContext' | 'onCheckedValueChange' | 'triggerId'>]) {
isUsingPropsAndContext = true;
}
}

return contextValue.hasMenuContext && isUsingPropsAndContext;
return hasMenuContext && isUsingPropsAndContext;
};
6 changes: 2 additions & 4 deletions packages/react-menu/src/contexts/menuContext.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import * as React from 'react';
import { createContext, useContextSelector, ContextSelector } from '@fluentui/react-context-selector';
import { createContext, useContextSelector, ContextSelector, Context } from '@fluentui/react-context-selector';
import { MenuListProps } from '../components/index';
import { MenuState } from '../components/Menu/index';

const MenuContext = createContext<MenuContextValue>({
export const MenuContext: Context<MenuContextValue> = createContext<MenuContextValue>({
open: false,
setOpen: () => false,
checkedValues: {},
onCheckedValueChange: () => null,
defaultCheckedValues: {},
hasMenuContext: false,
isSubmenu: false,
triggerRef: ({ current: null } as unknown) as React.MutableRefObject<HTMLElement>,
menuPopoverRef: ({ current: null } as unknown) as React.MutableRefObject<HTMLElement>,
Expand Down Expand Up @@ -42,7 +41,6 @@ export interface MenuContextValue
| 'inline'
> {
open: boolean;
hasMenuContext: boolean;
triggerId: string;
}

Expand Down
6 changes: 2 additions & 4 deletions packages/react-menu/src/contexts/menuListContext.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import * as React from 'react';
import { createContext, useContextSelector, ContextSelector } from '@fluentui/react-context-selector';
import { createContext, useContextSelector, ContextSelector, Context } from '@fluentui/react-context-selector';
import { SelectableHandler } from '../selectable/index';
import { MenuListProps } from '../components/index';

const MenuListContext = createContext<MenuListContextValue>({
export const MenuListContext: Context<MenuListContextValue> = createContext<MenuListContextValue>({
checkedValues: {},
onCheckedValueChange: () => null,
setFocusByFirstCharacter: () => null,
toggleCheckbox: () => null,
selectRadio: () => null,
hasIcons: false,
hasCheckmarks: false,
hasMenuListContext: false,
});

/**
Expand All @@ -22,7 +21,6 @@ export interface MenuListContextValue
setFocusByFirstCharacter?: (e: React.KeyboardEvent<HTMLElement>, itemEl: HTMLElement) => void;
toggleCheckbox?: SelectableHandler;
selectRadio?: SelectableHandler;
hasMenuListContext?: boolean;
}

export const MenuListProvider = MenuListContext.Provider;
Expand Down
7 changes: 4 additions & 3 deletions packages/react-menu/src/utils/useIsSubmenu.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { useHasParentContext } from '@fluentui/react-context-selector';
import { useMenuContext } from '../contexts/menuContext';
import { useMenuListContext } from '../contexts/menuListContext';
import { MenuListContext } from '../contexts/menuListContext';

/**
* A component can be a part of a submenu whether its menu context `isSubmenu` flag is true
Expand All @@ -11,7 +12,7 @@ import { useMenuListContext } from '../contexts/menuListContext';
*/
export function useIsSubmenu() {
const menuContextValue = useMenuContext(context => context.isSubmenu);
const menuListContextValue = useMenuListContext(context => context.hasMenuListContext);
const hasMenuListContext = useHasParentContext(MenuListContext);

return menuContextValue || menuListContextValue;
return menuContextValue || hasMenuListContext;
}

0 comments on commit fce57b2

Please sign in to comment.