material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[utils] Port `useLocalStorageState` hook from Toolpad

Open Janpot opened this issue 5 months ago • 2 comments

As per https://github.com/mui/mui-x/issues/11576#issuecomment-1893647652

  • Port useLocalStorageState hook from Toolpad.
  • Replace theme switcher implementation with this hook
  • Replace package manager switcher with this hook.

Immediate improvements this hook brings to the docs

  • Easier to reason about

  • Package manager now switches across the page

    https://github.com/mui/material-ui/assets/2109932/3020323d-65ed-433f-98b3-04e9919840f2

    https://deploy-preview-41096--material-ui.netlify.app/material-ui/getting-started/installation/

  • Theme now switches across opened tabs

    https://github.com/mui/material-ui/assets/2109932/2ff2b5fa-b2d2-4bcf-ab0e-82115c65d595

Janpot avatar Feb 14 '24 15:02 Janpot

Netlify deploy preview

https://deploy-preview-41096--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad) No bundle size changes

Generated by :no_entry_sign: dangerJS against b41efdcca1eb3ffb065d6d3edbdeceeb81cf2fac

mui-bot avatar Feb 14 '24 15:02 mui-bot

Changes like this one create an extra incentive to solve #35840.

oliviertassinari avatar Feb 14 '24 18:02 oliviertassinari

I was planning to replace our usage in toolpad with this implementation. X was also investigating whether they could provide datagrid persistence on top of it. I'm happy to move it wherever though, this seemed like a logical spot to me.

Janpot avatar Feb 20 '24 07:02 Janpot

X was also investigating whether they could provide datagrid persistence on top of it

What if it was Toolpad's scope to deliver this? Maybe that module could also have an adapter for TanStack Table, for AG Grid.

oliviertassinari avatar Feb 20 '24 20:02 oliviertassinari

This introduces regressions:

  1. The homepage dark mode toggle doesn't work anymore:
SCR-20240221-ndea

https://deploy-preview-41096--material-ui.netlify.app/

  1. The try/catch to support private mode was removed. To add back, e.g. https://github.com/mui/material-ui/pull/34027

oliviertassinari avatar Feb 21 '24 13:02 oliviertassinari

3rd regressions: which I can reproduce on https://deploy-preview-41096--material-ui.netlify.app/material-ui/react-button/ and in localhost:

https://github.com/mui/material-ui/assets/3165635/373fb830-3cb8-45a0-a9f3-b9a0d69e773f

For example: https://twitter.com/apoorv_taneja/status/1761292886052884955.

I have reverted with 0d086a17f9f85838a72ae31f4993d6e55bcc4b0a. I have done a follow-up on this, cleaning the logic https://github.com/mui/material-ui/pull/41223, which fixed the problem. I don't know exactly how, once I removed the redundant rerenders, the loop stopped but the dark/light mode was clearly broken. This was because Material UI and Material UI Next share the same dark/light locale storage value but also we were trying to sync Material UI Next light/dark mode with Material UI, which doesn't make sense if the locale storage value and DOM attributes are the same. I removed that and it works now.

oliviertassinari avatar Feb 24 '24 23:02 oliviertassinari