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

ButtonGroup overflow logic is running w/o overflowButton prop #1923

Closed
GerardasB opened this issue Mar 18, 2024 · 6 comments · Fixed by #1939
Closed

ButtonGroup overflow logic is running w/o overflowButton prop #1923

GerardasB opened this issue Mar 18, 2024 · 6 comments · Fixed by #1939
Assignees
Labels
bug Something isn't working

Comments

@GerardasB
Copy link

Describe the bug (current behavior)

ButtonGroup will re-render multiple times based on visible nodes returned by the useOverflow even when overflowButton prop is not provided. This complicates a use-case where a custom overflow logic is needed.

Expected Behavior

ButtonGroup should always render provided children as is when overflowButton is not specified.

Link to minimal repro

https://stackblitz.com/edit/github-fywssb?file=src%2FApp.tsx

Steps To Reproduce

  1. Open console of the repro
  2. Click Calculate overflow (simulates conditional rendering of overflow button)
  3. Observe that the overflow button is toggled correctly (render is fine)
  4. Observe the console - when the overflow button is rendered it logs HTMLCollection w/ 2 elements.

Anything else?

Final render is fine due to later re-renders.

@GerardasB GerardasB added the bug Something isn't working label Mar 18, 2024
@Ben-Pusey-Bentley
Copy link
Contributor

Thanks for submitting this issue. I can confirm that I am able to reproduce it using the steps that you have provided. I would like to add that the elements in the HTMLCollection are the 1 and 2 buttons, and the 3 button is missing, along with the overflow button. @mayank99 Do you know of a way that we could avoid this re-render?

@mayank99
Copy link
Contributor

This is strange. I actually noticed that all elements are correctly present in the HTMLCollection, even though the length is incorrect.

@mayank99
Copy link
Contributor

Fixed in 3.7.2

@HaveSpacesuit
Copy link
Contributor

@Ben-Pusey-Bentley Do you know if this would fix #1118?

@Ben-Pusey-Bentley
Copy link
Contributor

@Ben-Pusey-Bentley Do you know if this would fix #1118?

Did some testing with the linked stackblitz, and this change does not seem to have fixed that issue.

@mayank99
Copy link
Contributor

mayank99 commented Apr 4, 2024

#1118 is technically unrelated, and can be worked around by not using overflowButton. But it does present an interesting question: why is overflowButton so difficult to use in practice? Maybe we need to rethink how we've implemented useOverflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants