-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Update storybook #6563
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
base: master
Are you sure you want to change the base?
Update storybook #6563
Conversation
…e broken due to them not being kept up to date with components. all pages now display correctly and no console warnings/errors are printed (migration away from old-style required React prop-types was part of this). this should make quick QA verification possible; 2. storybook has been updated to eliminate some race conditions, which necessitated updating webpack, babel, and some older modules whose CommonJS exports were no longer acceptable. Some of these upgrade are incrementally toward updating react-admin (a future PR goal), so it's all good. 3. to clean up the webpack config, the browserlistrc has been updated to exclude pre-WebGL2 and pre-ES7 browsers. This should reduce the bundle size and have a runtime performance uplift since more built-ins will be used instead of JS fallbacks.
On my dev machine, this makes storybook run properly, yay! On my Hubs instance, the root page loads and run fine, and the admin panel is able to changes. But rooms do not run properly: https://hubs.hominidsoftware.com/v6tN9xi/treadmill-animals Under Firefox 142.0 (64-bit) (MacOS 15.6.1) the browser console has lots of errors like A resource is blocked by OpaqueResponseBlocking, please check browser console for details. frontend-b81f6f5c607d2acfec53.js A resource is blocked by OpaqueResponseBlocking, please check browser console for details. frontend-b81f6f5c607d2acfec53.js Under Chrome Version 139.0.7258.128 (Official Build) (x86_64) and Version 139.0.7258.139 (Official Build) (x86_64), the browser console has errors like Uncaught TypeError: Cannot set properties of undefined (setting 'workerSrc') pdf-system.ts:27 ...so the bundling may not have been done properly. |
…el, but this required bumping our minimum safari version to match pdfjs' constraints. I didn't update to latest so that we can still reasonably support Safari 14.1 (iPhone 6S, iPad Air 2, iPad mini 3), and Chrome 105 (Pico XR 4, Meta Quest 1). This excludes previous support for iOS 10 (iPhone 6, iPad Air) and Chrome 91 (Oculus Go). the typescript config was tightened slightly to try and surface this class of import/export mismatch at build-time in the future.
With commit 230bb84, Storybook still runs correctly, the root page, admin page and room pages load correctly. Changes in the admin panel take effect, Placing a PDF works correctly and the PDF can be paged through. Text chat works correctly. |
Imagineer & I checked that the changed components didn't regress in Hubs proper. I think all we need now, is validating that the changed Storybook stories render properly on Windows & Linux. |
Why: not functional without an account of our own
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthargett Found a couple bugs/theme regressions in the admin panel. Browser used: Firefox 136.
Current behaviour:
Screencast-2025-09-01_07.34.20.mp4
Screencast-2025-09-01_07.56.49.mp4
Expected behaviour:
Screencast-2025-09-01_07.34.41.mp4
Screencast-2025-09-01_08.02.01.mp4
This warning is now present in the console:
Material-UI: theme.mixins.gutters() is deprecated.
You can use the source of the mixin directly:
paddingLeft: theme.spacing(2),
paddingRight: theme.spacing(2),
[theme.breakpoints.up('sm')]: {
paddingLeft: theme.spacing(3),
paddingRight: theme.spacing(3),
},
… and put a visible vertical scrollbar on the content panel
- Logo stays fixed at top (never scrolls away) - Menu items scroll in dedicated area below logo - Single scroll area - no more double scrolling - Scroll indicators appear/disappear properly when content overflows - Works with Material-UI - no fighting the framework
What?
The main thrust of this PR is to update storybook and to fortify the existing storybook pages so that they can be relied upon for quicker, more accurate testing for follow-up PRs (e.g. upgrading react-admin).
Updating storybook to fix some async race issues that made it unreliable had some ripple effects.
a. .browserlistrc has been updated to include ES7-capable (and WebGL2) browsers only, eliminating the need for slow and problematic polyfills. See the comments in the file for explanation of the expansive coverage that still remains.
b. mixed CommonJS, UMD, and ES Modules that worked by some miracle were now no longer supported. This necessitated the minor upgrade of several packages: material-ui, react-redux, react-intl, pdfjs-dist, react-admin (not to latest, but just high enough to get ESM compatibility).
c. even so, not all sub-dependencies were possible to upgrade to ESM module compatibility, so there are still some manual overrides for them in the webpack.config. These can be removed once a bigger package upgrade leap is made.
Why?
Several storybook pages were no longer functional, as components were updated but their storybook pages were not. Many pages emitted a lot of console noise, making it hard to parse if a given change was "safe".
Examples
How to test
Bringing up the admin UI, make a change, click submit/save, and verify the change propagated correctly. Because most of these changes are bundling related it will very like either "just work" or fail immediately.
Enter a hubs space and check sharing PDF media, and typing messages in the chat sidebar.
When deploying from the branch, do it from a fresh checkout to make sure any issues reported aren't from other misc changes in your existing checkout :D
Documentation of functionality
There should be no functional change, except for perhaps more components "just working" correctly when OS/browser-level dark mode is enabled.
Limitations
This PR does not upgrade react-admin, html2canvas, hls-js, and others that are contributing to the list of critical vulnerabilities reported by
npm audit
.This PR does not unify everything under ESM. That, alongside a typescript upgrade, might be a good next "safe" PR that would help detect bugs earlier and make it easier for the project to accept contributions.
Alternatives considered
See PR #6549 , which this is an incremental carve out from.
Open questions
Additional details or related context