microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.33k stars 2.72k forks source link

[Bug]: v9 components do not render with style\theme on v8 surface components #24816

Closed sgbdev closed 2 years ago

sgbdev commented 2 years ago

Library

React Components / v9 (@fluentui/react-components)

System Info

System:
    OS: Windows 11
    CPU: (8) x64 Intel(R) Core(TM) i7-8665U CPU @ 1.90GHz
    Memory: 10.21 GB / 31.82 GB
  Browsers:
    Chrome: 105.0.5195.102

  npmPackages:
    @fluentui/react: 8.94.4
    @fluentui/react-components: 9.2.0

Are you reporting Accessibility issue?

no

Reproduction

https://codesandbox.io/s/fluentui-v9-theming-v8-surfaces-ttkq1x?file=/src/App.js

Bug Description

Actual Behavior

When attempting to render a FluentUI v9 component (like a Button), on a FluentUI v8 surface (Callout, Dialog, Panel, Modal), the v9 component renders without styles or theming. In the example provided, make sure the switches for "Use ApplyToBody" and "Use Layer Props" are set to No, and then click any of the example buttons on the right, to open the related v8 surface component. You'll notice the the v8 buttons display with the custom theme, while the v9 buttons do not (see the image below). image

Expected Behavior

I would expect the v9 buttons\components to render with styles and theming applied, just like the v8 components.

Additional Observations

There are other issues\feature requests that have been documented, while different, contain comments with potential workarounds for this issue ( see 23626 and 21290). Although, its not clear which solution is ideal. It would be great if this could be fixed with an out of the box solution. But, if that is not possible, then would you be able to clarify which of workaround you recommend?

Workarounds in Example

Apply To Body

When setting the "Use ApplyToBody" switch to Yes - the useEffect method that exists in that function calls useThemeClassName, and will apply those classes to the body element. The v9 component will render properly at this point. This is the easiest solution, but its drawbacks are document here .

v8 layerProps

When setting the "Use Layer Props" switch to Yes - the v8 surface components will target an element that is a child of the root FluentProvider. The v9 component will render properly at this point. This solution has its pro's and con's, depending on the implementation of Callout, Dialog, Panel, and Modal. If used within a wrapper component, then its just a matter of updating code in a few locations to implement this. If not, this could be a cumbersome solution, as it would require touching all instances of these components in an app.

Logs

No response

Requested priority

Normal

Products/sites affected

No response

Are you willing to submit a PR to fix?

yes

Validations

layershifter commented 2 years ago

Hey @sgbdev, thanks for reaching us and the detailed demo 👍

This problem is known and caused by missing CSS variables on portals created by @fluentui/react/@fluentui/react-northstar. However, this is solved by a package called @fluentui/react-portal-compat. Add PortalCompatProvider as an inner child of FluentProvider and it will work 😉

import { FluentProvider } from '@fluentui/react-components';
import { PortalCompatProvider } from '@fluentui/react-portal-compat';

function App() {
  return (
    <FluentProvider>
      <PortalCompatProvider>{/* your components */}</PortalCompatProvider>
    </FluentProvider>
  );
}

https://codesandbox.io/s/fluentui-v9-theming-v8-surfaces-forked-umthe4?file=/src/index.js


Do you have any suggestions where we can put this in documentation to make it more discoverable? 🤔

sgbdev commented 2 years ago

This is exactly what I was looking for! Thank you so much!

Regarding discoverability in the documentation.

  1. A dedicated page in the Components group, or perhaps in another group called Utilities (similar to the v8 docs).
  2. Maybe add a note to the Theming page, in the How theme is applied section. Not sure if that's what the intention is with the current link to the Portal component. But, that link does not lead anywhere.
  3. Perhaps adding another example on the FluentProvider component page , with a note explaining its applicability if using existing @fluentui/react and @fluentui/react-northstar libraries. This is where I initially went to see if there was a solution.
  4. It may also be worth mentioning on the Troubleshooting or Theme Migration pages.
layershifter commented 1 year ago

@sgbdev Thanks for feedback, I pushed few changes in #24951 👍