diff --git a/change/@fluentui-react-portal-compat-89771dd5-317e-4567-a861-92ddaecdbfdd.json b/change/@fluentui-react-portal-compat-89771dd5-317e-4567-a861-92ddaecdbfdd.json new file mode 100644 index 00000000000000..99798b87c8f4dc --- /dev/null +++ b/change/@fluentui-react-portal-compat-89771dd5-317e-4567-a861-92ddaecdbfdd.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: handle multiple classes in PortalCompatProvider", + "packageName": "@fluentui/react-portal-compat", + "email": "olfedias@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-portal-compat/src/PortalCompatProvider.test.tsx b/packages/react-components/react-portal-compat/src/PortalCompatProvider.test.tsx index 987b5cebbc0f52..f34e55f04d66f2 100644 --- a/packages/react-components/react-portal-compat/src/PortalCompatProvider.test.tsx +++ b/packages/react-components/react-portal-compat/src/PortalCompatProvider.test.tsx @@ -1,31 +1,37 @@ import { ThemeClassNameProvider_unstable as ThemeClassNameProvider } from '@fluentui/react-shared-contexts'; import { usePortalCompat } from '@fluentui/react-portal-compat-context'; -import { FluentProvider } from '@fluentui/react-provider'; +import { FluentProvider, useFluentProviderThemeStyleTag } from '@fluentui/react-provider'; import { IdPrefixProvider, resetIdsForTests } from '@fluentui/react-utilities'; import { renderHook } from '@testing-library/react-hooks'; import * as React from 'react'; -import { PortalCompatProvider } from './PortalCompatProvider'; +import { PortalCompatProvider, useProviderThemeClasses } from './PortalCompatProvider'; // eslint-disable-next-line @typescript-eslint/no-empty-function const noop = () => {}; -describe('PortalCompatProvider', () => { - afterEach(() => { - resetIdsForTests(); +const TestWrapperWithMultipleClasses: React.FC = props => { + // Creates a second className with CSS variables + const { styleTagId } = useFluentProviderThemeStyleTag({ + theme: { borderRadiusCircular: '50px' }, + targetDocument: document, + rendererAttributes: {}, }); - it('registers a function in a context', () => { - jest.spyOn(console, 'warn').mockImplementation(noop); - - const { result } = renderHook(() => usePortalCompat(), { wrapper: PortalCompatProvider }); + return ( + + {props.children} + + ); +}; - expect(result.current).toBeInstanceOf(Function); +describe('useProviderThemeClasses', () => { + afterEach(() => { + resetIdsForTests(); }); - it('during register adds a className from "ThemeClassNameContext" context', () => { - const element = document.createElement('div'); - const { result } = renderHook(() => usePortalCompat(), { + it('handles classes from FluentProvider', () => { + const { result } = renderHook(() => useProviderThemeClasses(), { wrapper: props => ( {props.children} @@ -33,17 +39,28 @@ describe('PortalCompatProvider', () => { ), }); - expect(result.current(element)).toBeInstanceOf(Function); - expect(element.classList).toMatchInlineSnapshot(` - DOMTokenList { - "0": "fui-FluentProvider1", - } + expect(result.current).toMatchInlineSnapshot(` + Array [ + "fui-FluentProvider1", + ] `); }); - it('during register adds a className from "ThemeClassNameContext" context with custom ID prefix', () => { - const element = document.createElement('div'); - const { result } = renderHook(() => usePortalCompat(), { + it('handles multiple classes from FluentProvider', () => { + const { result } = renderHook(() => useProviderThemeClasses(), { + wrapper: TestWrapperWithMultipleClasses, + }); + + expect(result.current).toMatchInlineSnapshot(` + Array [ + "fui-FluentProvider2", + "fui-FluentProvider1", + ] + `); + }); + + it('handles classes with custom ID prefix', () => { + const { result } = renderHook(() => useProviderThemeClasses(), { wrapper: props => ( @@ -53,28 +70,95 @@ describe('PortalCompatProvider', () => { ), }); + expect(result.current).toMatchInlineSnapshot(` + Array [ + "custom1-fui-FluentProvider1", + ] + `); + }); + + it('handles classes with a React 18 compatible ID', () => { + const { result } = renderHook(() => useProviderThemeClasses(), { + wrapper: props => ( + + {props.children} + + ), + }); + + expect(result.current).toMatchInlineSnapshot(` + Array [ + "fui-FluentProviderR1a", + ] + `); + }); + + it('returns only proper classes', () => { + const { result } = renderHook(() => useProviderThemeClasses(), { + wrapper: props => ( + + {props.children} + + ), + }); + + expect(result.current).toHaveLength(0); + }); + + it('logs a warning when does not have top level FluentProvider', () => { + const warn = jest.fn().mockImplementation(noop); + jest.spyOn(console, 'warn').mockImplementation(warn); + + renderHook(() => useProviderThemeClasses(), { wrapper: PortalCompatProvider }); + + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('PortalCompatProvider: "useThemeClassName()" hook returned an empty string'), + ); + }); +}); + +describe('PortalCompatProvider', () => { + afterEach(() => { + resetIdsForTests(); + }); + + it('registers a function in a context', () => { + jest.spyOn(console, 'warn').mockImplementation(noop); + + const { result } = renderHook(() => usePortalCompat(), { wrapper: PortalCompatProvider }); + + expect(result.current).toBeInstanceOf(Function); + }); + + it('during register adds a className from "ThemeClassNameContext" context', () => { + const element = document.createElement('div'); + const { result } = renderHook(() => usePortalCompat(), { + wrapper: props => ( + + {props.children} + + ), + }); + expect(result.current(element)).toBeInstanceOf(Function); expect(element.classList).toMatchInlineSnapshot(` DOMTokenList { - "0": "custom1-fui-FluentProvider1", + "0": "fui-FluentProvider1", } `); }); - it('during register adds a className from "ThemeClassNameContext" context with a React 18 compatible ID', () => { + it('during register adds multiple classes from "ThemeClassNameContext" context if they exist', () => { const element = document.createElement('div'); const { result } = renderHook(() => usePortalCompat(), { - wrapper: props => ( - - {props.children} - - ), + wrapper: TestWrapperWithMultipleClasses, }); expect(result.current(element)).toBeInstanceOf(Function); expect(element.classList).toMatchInlineSnapshot(` DOMTokenList { - "0": "fui-FluentProviderR1a", + "0": "fui-FluentProvider2", + "1": "fui-FluentProvider1", } `); }); @@ -101,29 +185,4 @@ describe('PortalCompatProvider', () => { expect(unregister()).toBeUndefined(); expect(element.classList.length).toBe(0); }); - - it('during register adds only proper className', () => { - const element = document.createElement('div'); - const { result } = renderHook(() => usePortalCompat(), { - wrapper: props => ( - - {props.children} - - ), - }); - result.current(element); - - expect(element.classList.length).toBe(0); - }); - - it('logs a warning when does not have top level FluentProvider', () => { - const warn = jest.fn().mockImplementation(noop); - jest.spyOn(console, 'warn').mockImplementation(warn); - - renderHook(() => usePortalCompat(), { wrapper: PortalCompatProvider }); - - expect(warn).toHaveBeenCalledWith( - expect.stringContaining('PortalCompatProvider: "useThemeClassName()" hook returned an empty string'), - ); - }); }); diff --git a/packages/react-components/react-portal-compat/src/PortalCompatProvider.tsx b/packages/react-components/react-portal-compat/src/PortalCompatProvider.tsx index b14a635a36f126..337a51e89789f2 100644 --- a/packages/react-components/react-portal-compat/src/PortalCompatProvider.tsx +++ b/packages/react-components/react-portal-compat/src/PortalCompatProvider.tsx @@ -5,39 +5,17 @@ import { applyFocusVisiblePolyfill } from '@fluentui/react-tabster'; import type { RegisterPortalFn } from '@fluentui/react-portal-compat-context'; -const CLASS_NAME_REGEX = new RegExp(`([^\\s]*${fluentProviderClassNames.root}\\w+)`); - -export const PortalCompatProvider: React.FC<{ children?: React.ReactNode }> = props => { - const { children } = props; +const CLASS_NAME_REGEX = new RegExp(`([^\\s]*${fluentProviderClassNames.root}\\w+)`, 'g'); +export function useProviderThemeClasses(): string[] { const themeClassName = useThemeClassName(); - const cssVariablesClassName = React.useMemo( - // "themeClassName" may contain multiple classes while we want to add only a class that hosts CSS variables + const cssVariablesClasses = React.useMemo( + // "themeClassName" may contain multiple classes while we want to add only classes that host CSS variables // Keep in sync with "packages/react-provider/src/components/FluentProvider/useFluentProviderThemeStyleTag.ts" - () => themeClassName.match(CLASS_NAME_REGEX)?.[1], + () => themeClassName.match(CLASS_NAME_REGEX) ?? [], [themeClassName], ); - const registerPortalEl = React.useCallback( - element => { - let disposeFocusVisiblePolyfill: () => void = () => undefined; - if (cssVariablesClassName) { - element.classList.add(cssVariablesClassName); - if (element.ownerDocument.defaultView) { - disposeFocusVisiblePolyfill = applyFocusVisiblePolyfill(element, element.ownerDocument.defaultView); - } - } - - return () => { - if (cssVariablesClassName) { - element.classList.remove(cssVariablesClassName); - } - disposeFocusVisiblePolyfill(); - }; - }, - [cssVariablesClassName], - ); - if (process.env.NODE_ENV !== 'production') { // This if statement technically breaks the rules of hooks, but ENV variables never change during app lifecycle // eslint-disable-next-line react-hooks/rules-of-hooks @@ -54,5 +32,29 @@ export const PortalCompatProvider: React.FC<{ children?: React.ReactNode }> = pr }, []); } + return cssVariablesClasses; +} + +export const PortalCompatProvider: React.FC<{ children?: React.ReactNode }> = props => { + const { children } = props; + const cssVariablesClasses = useProviderThemeClasses(); + + const registerPortalEl = React.useCallback( + element => { + let disposeFocusVisiblePolyfill: () => void = () => undefined; + + element.classList.add(...cssVariablesClasses); + if (element.ownerDocument.defaultView) { + disposeFocusVisiblePolyfill = applyFocusVisiblePolyfill(element, element.ownerDocument.defaultView); + } + + return () => { + element.classList.remove(...cssVariablesClasses); + disposeFocusVisiblePolyfill(); + }; + }, + [cssVariablesClasses], + ); + return {children}; };