tooltip: stop using <Component /> to better support closures

Summary:
I noticed that the scrollbar state or `useState` state in the <Tooltip /> dropdown
does not preserve across state changes. But they preserve fine without <Tooltip />.

I think what happens is, in this function:

  function PartialSelectionAction({file}: {file: UIChangedFile}) {
    const getPanel = () => {
      return <PartialSelectionPanel file={file} />;
    };

    return (
      <Tooltip
        component={getPanel}
        ...>
      </Tooltip>
    );
  }

The `getPanel` is a dynamically created function, and gets used by Tooltip like
`<getPanel ... />`. Then React notices that `getPanel` changes every time per
render (because `getPanel` is not a "static" function) and treat the `getPanel`
as "incompatible" component and re-render the whole thing. Note the `getPanel`
needs the `file` in the closure, so it cannot be a static function.

Fix this issue by avoiding treating `component` as a React component (so React
won't treat it as "component type changed" when re-rendering).

To avoid misuse, I updated `component` signature to no longer be compatible
with a React component. Callsites need to migrate `component={Foo}` to
`component={(dismiss) => <Foo dismiss={dismiss} />}`.

Reviewed By: evangrayk

Differential Revision: D46244645

fbshipit-source-id: 40ea4c0d6637da19ac9fe2b23d9c334e2575a148
This commit is contained in:
Jun Wu 2023-05-31 12:45:12 -07:00 committed by Facebook GitHub Bot
parent 991c8e0664
commit 75e38e362c
4 changed files with 10 additions and 7 deletions

View File

@ -24,7 +24,10 @@ import './BugButton.css';
export function BugButton() {
return (
<MaybeBugButtonNux>
<Tooltip trigger="click" component={BugDropdown} placement="bottom">
<Tooltip
trigger="click"
component={dismiss => <BugDropdown dismiss={dismiss} />}
placement="bottom">
<VSCodeButton appearance="icon" data-testid="bug-button">
<Icon icon="bug" />
</VSCodeButton>

View File

@ -52,7 +52,7 @@ export function CwdSelector() {
}
const repoBasename = basename(info.repoRoot);
return (
<Tooltip trigger="click" component={CwdDetails} placement="bottom">
<Tooltip trigger="click" component={() => <CwdDetails />} placement="bottom">
<VSCodeButton appearance="icon" data-testid="cwd-dropdown-button">
<Icon icon="folder" slot="start" />
{repoBasename}

View File

@ -30,7 +30,7 @@ import './SettingsTooltip.css';
export function SettingsGearButton() {
return (
<Tooltip trigger="click" component={SettingsDropdown} placement="bottom">
<Tooltip trigger="click" component={() => <SettingsDropdown />} placement="bottom">
<VSCodeButton appearance="icon" data-testid="settings-gear-button">
<Icon icon="gear" />
</VSCodeButton>

View File

@ -33,8 +33,8 @@ type TooltipProps = {
delayMs?: number;
} & ExclusiveOr<
ExclusiveOr<{trigger: 'manual'; shouldShow: boolean}, {trigger?: 'hover' | 'disabled'}> &
ExclusiveOr<{component: (props: {dismiss: () => void}) => JSX.Element}, {title: string}>,
{trigger: 'click'; component: (props: {dismiss: () => void}) => JSX.Element; title?: string}
ExclusiveOr<{component: (dismiss: () => void) => JSX.Element}, {title: string}>,
{trigger: 'click'; component: (dismiss: () => void) => JSX.Element; title?: string}
>;
type VisibleState =
@ -68,7 +68,7 @@ type VisibleState =
export function Tooltip({
children,
title,
component: Component,
component,
placement: placementProp,
trigger: triggerProp,
delayMs,
@ -82,7 +82,7 @@ export function Tooltip({
if (visible === 'title') {
return title;
}
return Component == null ? title : <Component dismiss={() => setVisible(false)} />;
return component == null ? title : component(() => setVisible(false));
};
useEffect(() => {