Skip to content

Conversation

matthargett
Copy link
Contributor

@matthargett matthargett commented Aug 21, 2025

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.

  1. webpack and babel needed to be updated, and this caused issues with ancient polyfills. To resolve that issue..
    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.
  2. It seems that there is better Dark Mode support as a side effect
  3. Storybook 9.x makes for easier comparison of different device resolution (and locales), so rapid visual testing with broader coverage is much easier.
  4. the typescript config was gently expanded to help find classes of issues that led to the first bug report against this PR. expanding typescript strict mode should probably be the next PR to keep maintenance and test costs low.

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

…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.
@DougReeder
Copy link
Contributor

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
GET
https://assets.hubs.hominidsoftware.com/hubs/assets/js/frontend-b81f6f5c607d2acfec53.js
NS_BINDING_ABORTED

A resource is blocked by OpaqueResponseBlocking, please check browser console for details. frontend-b81f6f5c607d2acfec53.js
Loading failed for the <script> with source “https://assets.hubs.hominidsoftware.com/hubs/assets/js/frontend-b81f6f5c607d2acfec53.js”. treadmill-animals:100:601

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.
@DougReeder
Copy link
Contributor

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.

@DougReeder
Copy link
Contributor

DougReeder commented Aug 26, 2025

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.

matthargett and others added 2 commits August 30, 2025 10:51
Why: not functional without an account of our own
Copy link
Contributor

@Exairnous Exairnous left a 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),
      },

  - 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
@DougReeder
Copy link
Contributor

I'm seeing dead space at the top:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants