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-infra] Product switcher popup not closing #41128

Closed
oliviertassinari opened this issue Feb 16, 2024 · 5 comments · Fixed by #41166
Closed

[docs-infra] Product switcher popup not closing #41128

oliviertassinari opened this issue Feb 16, 2024 · 5 comments · Fixed by #41166
Labels
bug 🐛 Something doesn't work ready to take Help wanted. Guidance available. There is a high chance the change will be accepted scope: docs-infra Specific to the docs-infra product

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 16, 2024

Steps to reproduce

Steps:

  1. Open https://master--material-ui.netlify.app/material-ui/react-button/
  2. Try to go to Base UI docs
  3. Popup stays open
Screenshot 2023-08-23 at 23 39 58

Context

This is because of the use of a layout, related to #41117.

Your environment

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: -

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work scope: docs-infra Specific to the docs-infra product labels Feb 16, 2024
@divyammadhok
Copy link
Contributor

Hey @oliviertassinari I can work on resolving this bug. It would be great if you can assign this to me.

@danilo-leal danilo-leal changed the title [docs-infra] Docs popup not closing [docs-infra] Product switcher popup not closing Feb 16, 2024
@danilo-leal danilo-leal moved this to Backlog in Docs-infra Feb 16, 2024
@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Feb 16, 2024
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Feb 16, 2024

This should be a simple bug to fix but one that will be increasingly more annoying as we improve the use of "layouts" in our docs to improve performance.

@divyammadhok Go ahead, thanks

@divyammadhok
Copy link
Contributor

Hey @oliviertassinari I added a PR for this issue - 41128
Now there were 2 approaches - either track for page changes and close the popper or pass the close callback through props and call it whenever the links are clicked but the 2nd approach felt a bit more manual to handle. Every time a new link / component is added we would need to handle for this handleClose callback so I instead went ahead with the 1st approach by tracking page changes.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Feb 18, 2024

Same bug, the user feedback is broken, it should close when going to the same area:

Screen.Recording.2024-02-18.at.15.02.15.mov

https://mui.com/material-ui/getting-started/


Off-topic wonder if we shouldn't open in a new tab as the Google app switcher works:

SCR-20240218-njlx

@divyammadhok
Copy link
Contributor

divyammadhok commented Feb 18, 2024

@oliviertassinari
Ah Gotcha, so for the same tab as the opened route we need it to close, then I believe the best path forward would be to pass an event to close it whenever it is clicked and treat every item as a close button.


Yeah, but for Google app switcher I believe the motivation is that it's possible a user is editing on the current tab but for MUI docs it's more of a readonly documentation, so not sure, if opening in new tab good for UX. Also you can check this accessibility guideline for redirection in new tab https://www.w3.org/TR/WCAG20-TECHS/G201.html Ideally from a better accessibility perspective it's advised to open in same tab as opening in new tab takes away from the ability to use back to continue where they were previously.

Edit: Kindly check the PR once more, have updated the logic.

@github-project-automation github-project-automation bot moved this from Backlog to Done in Docs-infra Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work ready to take Help wanted. Guidance available. There is a high chance the change will be accepted scope: docs-infra Specific to the docs-infra product
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants