This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 49289be:

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
react-components react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	165.87 kB 46.955 kB	165.697 kB 46.873 kB	-173 B -82 B
react-menu Menu (including children components)	103 kB 31.806 kB	102.827 kB 31.706 kB	-173 B -100 B
react-menu Menu (including selectable components)	105.251 kB 32.167 kB	105.078 kB 32.072 kB	-173 B -95 B
react-tooltip Tooltip	42.008 kB 14.478 kB	42.289 kB 14.589 kB	281 B 111 B

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 332d8f47ddea3daa577c95220a11f8bf5a7ac365 (build)

Perf Analysis (@fluentui/react)

No significant results to display.

Perf Analysis (@fluentui/react-northstar)

⚠️ No perf measurements available

Add an example of composition

IMO we should mention this behavior in docs somehow, via an example or something else.

const CallButton = React.forwardRef((props, ref) => {
  return (
    <Tooltip content="bar" relationship="label">
      <Button
        {...props}
        ref={ref}
        onClick={(...args) => {
          props.onClick(...args);
          // custom logic
        }}
      >
        Toggle menu
      </Button>
    </Tooltip>
  );
});

export const MenuTriggerWithTooltip = () => (
  <Menu>
    <MenuTrigger>
      <CallButton />
    </MenuTrigger>

    <MenuPopover>
      <MenuList>
        <MenuItem>New </MenuItem>
        <MenuItem>New Window</MenuItem>
        <MenuItem disabled>Open File</MenuItem>
        <MenuItem>Open Folder</MenuItem>
      </MenuList>
    </MenuPopover>
  </Menu>
);

This example works fully with current approach, but there are few things that worth mentioning:

• React.forwardRef() is required
  
• props & ref should be passed to a Button (or other component that behaves like a trigger)
• custom callbacks should call matching handlers from props

@layershifter

IMO we should mention this behavior in docs somehow, via an example or something else.

[...]

This example works fully with current approach, but there are few things that worth mentioning:

1. React.forwardRef() is required
2. props & ref should be passed to a Button (or other component that behaves like a trigger)
3. custom callbacks should call matching handlers from props

We might want to document FluentTriggerComponent, and how to write a custom component that acts as a trigger (or, at least, mention that you have to add isFluentTriggerComponent = true if you extend Tooltip or something).

But I don't see anything in your example that needs explicit documentation IMO:

1. React.forwardRef() is always required when forwarding a ref, regardless of whether they add a tooltip or not.
2. That's the case for any higher order component that goes in place of the thing its wrapping: it needs to pass the props to the inner component that it wraps. There's nothing special about triggers here. One thing that actually helps is that the Tooltip component doesn't allow any of those props on it, so it at least makes the author think more about where they should be forwarding their props to.
3. If they're ignoring some things on props, like the event handlers, then that's a bug in their component regardless of whether it's used as a trigger target or not.

I suppose if anything, the potential confusing part is that MenuTrigger and Tooltip "magically" set some HTMLElement props on their child, and those props need to work as expected. We could theoretically add something that mentions that to the documentation for those components.

@layershifter I created PR #21532 to fix the aria-label overriding issue

We might want to document FluentTriggerComponent, and how to write a custom component that acts as a trigger (or, at least, mention that you have to add isFluentTriggerComponent = true if you extend Tooltip or something).

I am not sure that it is applicable to the example that I posted as CallButton does not have children 🙁

It will only work for scenarios when someone composes Tooltip or *Trigger components in this way:

const LabelTooltip = (props) => {
  return (
    <Tooltip {...props} relationship="label" />
   );
}

function App() {
  return (
    <LabelTooltip>
      <button />
   </LabelTooltip>
  )
}

I think that isFluentTriggerComponent should be documented anyway as components could also composed with hooks pattern.

@layershifter

I am not sure that it is applicable to the example that I posted as CallButton does not have children 🙁

That's right: I was mentioning documenting FluentTriggerComponent for something like the LabelTooltip example you gave.

In the CallButton example, it would be up to CallButton to handle prop merging and forwarding itself (like you're doing in the example). That's the same as with any higher-order component, and isn't special for triggers, or even special to our library. That's what I was explaining with the numbered list in my reply.