microsoft / fluentui

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

[Bug]: InfoLabel does not respect `mountNode` property #31242

Open brhallin opened 2 weeks ago

brhallin commented 2 weeks ago

Library

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

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (12) x64 Intel(R) Xeon(R) W-2235 CPU @ 3.80GHz
    Memory: 36.24 GB / 63.59 GB
  Browsers:
    Edge: Chromium (123.0.2420.97)
    Internet Explorer: 11.0.22621.1

Are you reporting Accessibility issue?

None

Reproduction

https://stackblitz.com/edit/4bnprj?file=src%2Fexample.tsx

Bug Description

Actual Behavior

When clicked, the InfoLabel popover component should be attached to the DOM as a child of the element passed to the control, e.g.

<InfoLabel
  {...props}
  infoButton={{ popover: { mountNode: popoverRoot } }}
/>

Expected Behavior

When clicked, the InfoLabel popover component is attached to the DOM as a sibling of its <button>.

Note that this behaviour was previously supported, albeit in a slightly different fashion, back when the component was simply known as InfoButton, via its popover property, e.g. <InfoButton {...rest} popover={{ mountNode: popoverRoot }} />.

In the provided minimal repro, we can see that the mountNode property provided does indeed point to a valid DOM element.

Logs

No response

Requested priority

Blocking

Products/sites affected

PowerApps Canvas InfoButton component

Are you willing to submit a PR to fix?

yes

Validations

Shubhdeep12 commented 3 days ago

Hi @brhallin @Hotell Can I try resolving this bug?

It appears there are two issues:

  1. divRef.current is passed as null because after the initial render, when divRef gets its value, the component does not re-render.
  2. The infoLabel is not functioning correctly when passing mountNode in the infoButton prop. The reasons are: a. The infoLabel uses infoButton, which in turn uses PopOver. b. The PopOver surface uses Portal when the surface is not inline. c. In useInfoButton.tsx, the inline prop has a default value of true, which is causing the issue. d. We should either pass inline as false or have inline automatically set to false when mountNode is passed.

Thanks!

brhallin commented 2 days ago

Thanks for investigating the issue and identifying the problem!

We should either pass inline as false or have inline automatically set to false when mountNode is passed.

That's entirely up to the Fluent team, so I won't comment on the best decision. My team, however, now has a way forward -- we'll just add inline: false along with the mountNode prop, and we'll be unblocked.

🍻

Shubhdeep12 commented 2 days ago

Sure but we are halfway.

on passinginline: false along with mountNode in infoButton, do create the infoLabel popover inside the mountNode element, but the positioning of the popover is still at infoButton.

Reference: popover is created and gets added in mountNode div element but still shown above the infoButton image

brhallin commented 2 days ago

FWIW, in our use case, with the Canvas environment, the mountNode property we're providing points to a valid DOM element and with the addition of inline: false, it's now respecting that element.

image
brhallin commented 2 days ago

It's possible that the minimal demo I provided is faulty.

Shubhdeep12 commented 2 days ago

great!