Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Docs]: Document that Overflow components need to be wrapped around components that forward ref #27652

Closed
2 tasks done
flora8984461 opened this issue Apr 21, 2023 · 6 comments · Fixed by #31292
Closed
2 tasks done

Comments

@flora8984461
Copy link
Contributor

Library

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

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (12) x64 Intel(R) Xeon(R) W-2133 CPU @ 3.60GHz
    Memory: 51.85 GB / 63.57 GB
  Browsers:
    Edge: Spartan (44.22621.1105.0), Chromium (109.0.1518.78)
    Internet Explorer: 11.0.22621.1

Are you reporting Accessibility issue?

None

Reproduction

https://codesandbox.io/s/wandering-morning-to0rl6?file=/example.tsx:2082-2106

Bug Description

Actual Behavior

I am trying to make customized component that is being wrapped by OverflowItem.

The code looks like:

//  customized component
const ItemVisible = (props) => {
  const isVisible = useIsOverflowItemVisible(props.id);

  console.log("Item is visible", isVisible);
  return <Button>Item {props.id}</Button>;
};

export const Default = () => {
  const styles = useStyles();

  const itemIds = new Array(8).fill(0).map((_, i) => i.toString());

  return (
    <Overflow>
      <div className={mergeClasses(styles.container, styles.resizableArea)}>
        {itemIds.map((i) => (
          <OverflowItem key={i} id={i}>
            <ItemVisible id={i} /> // customized component
          </OverflowItem>
        ))}

        <OverflowMenu itemIds={itemIds} />
      </div>
    </Overflow>
  );
};

Because I want to use useIsOverflowItemVisible in the customized ItemVisible component to do some extra behavior. However, I got the error in the console:

Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Check the render method of `ForwardRef`.
    at ItemVisible (https://to0rl6.csb.app/example.tsx:103:74)
    at eval (https://to0rl6.csb.app/node_modules/@fluentui/react-overflow/lib-commonjs/components/OverflowItem/OverflowItem.js:14:13)
    at div
    at Provider (https://to0rl6.csb.app/node_modules/@fluentui/react-context-selector/lib-commonjs/createContext.js:16:46)
    at eval (https://to0rl6.csb.app/node_modules/@fluentui/react-overflow/lib-commonjs/components/Overflow.js:29:20)
    at Default (https://to0rl6.csb.app/example.tsx:115:18)
    at div
    at TextDirectionProvider (https://to0rl6.csb.app/node_modules/@griffel/react/TextDirectionContext.esm.js:23:33)
    at eval (https://to0rl6.csb.app/node_modules/@fluentui/react-provider/lib-commonjs/components/FluentProvider/FluentProvider.js:16:69)

And the overflow does not work, useIsOverflowItemVisible always return false even the items are visible.

Expected Behavior

I would expect there is no error, and useIsOverflowItemVisible can be used and return correct result in customized component that is wrapped in OverflowItem.

Logs

Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Check the render method of `ForwardRef`.
    at ItemVisible (https://to0rl6.csb.app/example.tsx:103:74)
    at eval (https://to0rl6.csb.app/node_modules/@fluentui/react-overflow/lib-commonjs/components/OverflowItem/OverflowItem.js:14:13)
    at div
    at Provider (https://to0rl6.csb.app/node_modules/@fluentui/react-context-selector/lib-commonjs/createContext.js:16:46)
    at eval (https://to0rl6.csb.app/node_modules/@fluentui/react-overflow/lib-commonjs/components/Overflow.js:29:20)
    at Default (https://to0rl6.csb.app/example.tsx:115:18)
    at div
    at TextDirectionProvider (https://to0rl6.csb.app/node_modules/@griffel/react/TextDirectionContext.esm.js:23:33)
    at eval (https://to0rl6.csb.app/node_modules/@fluentui/react-provider/lib-commonjs/components/FluentProvider/FluentProvider.js:16:69)

Requested priority

High

Products/sites affected

No response

Are you willing to submit a PR to fix?

yes

Validations

  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@flora8984461 flora8984461 changed the title [Bug]: useIsOverflowItemVisible does not work with item that is not in overflow [Bug]: useIsOverflowItemVisible does not work with item wrapped in Overflow Apr 21, 2023
@flora8984461 flora8984461 changed the title [Bug]: useIsOverflowItemVisible does not work with item wrapped in Overflow [Bug]: useIsOverflowItemVisible does not work with item wrapped in OverflowItem Apr 21, 2023
@ling1726 ling1726 changed the title [Bug]: useIsOverflowItemVisible does not work with item wrapped in OverflowItem [Docs]: Document that Overflow components need to be wrapped around components that forward ref Apr 24, 2023
@ling1726
Copy link
Member

@flora8984461, this happens because the component that OverflowItem wraps needs to be able to forward refs. In your case you could simply do

//  customized component
const ItemVisible = React.forwardRef((props, ref) => {
  const isVisible = useIsOverflowItemVisible(props.id);

  console.log("Item is visible", isVisible);
  return <Button ref={ref}>Item {props.id}</Button>;
});

export const Default = () => {
  const styles = useStyles();

  const itemIds = new Array(8).fill(0).map((_, i) => i.toString());

  return (
    <Overflow>
      <div className={mergeClasses(styles.container, styles.resizableArea)}>
        {itemIds.map((i) => (
          <OverflowItem key={i} id={i}>
            <ItemVisible id={i} /> // customized component
          </OverflowItem>
        ))}

        <OverflowMenu itemIds={itemIds} />
      </div>
    </Overflow>
  );
};

https://codesandbox.io/s/funny-davinci-c29hbm?file=/example.tsx

@ling1726
Copy link
Member

Keeping this issue open as a documentation issue

@flora8984461
Copy link
Contributor Author

@ling1726 thank you! The part makes OverflowItem needs ref being forwarded is it here? The ref is being used to render child element?

https://github.com/microsoft/fluentui/blob/master/packages/react-components/react-overflow/src/components/OverflowItem/OverflowItem.tsx#L15

@microsoft-github-policy-service

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Resolution: Soft Close Soft closing inactive issues over a certain period label Sep 21, 2023
@microsoft-github-policy-service

Because this issue has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

@ling1726 ling1726 reopened this Sep 25, 2023
@ling1726 ling1726 added Package: priority-overflow and removed Resolution: Soft Close Soft closing inactive issues over a certain period labels Sep 25, 2023
@barbalex
Copy link

barbalex commented Jan 14, 2024

I spent a half hour trying to get Overflow components work. Finally found this crucial information. Please add it to the docs.
Thanks for this great tool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment