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.
@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
… broken. fix height calculation issue that clipped the logo image incorrectly. verified bottom arrow still rendering correctly.
Initially it's fine. After any scrolling of either pane that doesn't leave the menu scrolled up to show lower items, the top of the page looks like the screenshot. :-S |
got it. fixed the storybook to use the production image, which allowed me to reproduce locally. fixed the issue, both sidebar and main content panel now scroll up and down and the image height never gets truncated in Firefox 143 and Chrome Canary 142. |
|
i've tried fixing this new problem a few different ways, but it all ends up fighting the framework and breaking other stuff. this will have to wait until react-admin v4+, and there are discussions elsewhere about the challenges with even this version we updated to. I tried making a bigger leap forward, which would give us properties on the react-admin controls to fine-tune this in a more straightforward way, but it yanks many other dependencies forward and we'd be back at the "big bang" PR #6549 . I did try this with Claude Code, OpenAI Codex, and others as well. they all chase their tail trying to get this done within the constraints. |
What: 1. Removes much of the AI styling changes. 2. Changes the custom admin theme to inherit properties from the default theme. 3. Removes the sidebar text color override for mobile. 4. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 5. Restores the appBar hiding functionality for desktop/large screens. 6. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 3. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 4. It looks better to have the sidebar be a single consistent color from top to bottom. 5. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 6. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization
What: 1. Removes much of the AI styling changes. 2. Changes the custom admin theme to inherit properties from the default theme. 3. Removes the sidebar text color override for mobile. 4. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 5. Restores the appBar hiding functionality for desktop/large screens. 6. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 3. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 4. It looks better to have the sidebar be a single consistent color from top to bottom. 5. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 6. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization
What: 1. Removes much of the AI styling changes. 2. Changes the custom admin theme to inherit properties from the default theme. 3. Removes the sidebar text color override for mobile. 4. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 5. Restores the appBar hiding functionality for desktop/large screens. 6. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 3. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 4. It looks better to have the sidebar be a single consistent color from top to bottom. 5. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 6. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization
What: 1. Removes much of the AI styling changes. 2. Changes the custom admin theme to inherit properties from the default theme. 3. Removes the sidebar text color override for mobile. 4. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 5. Restores the appBar hiding functionality for desktop/large screens. 6. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 3. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 4. It looks better to have the sidebar be a single consistent color from top to bottom. 5. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 6. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization
What: 1. Removes much of the AI styling changes. 2. Reverts a21f8d4 (show production hubs logo in the storybook...). 3. Changes the custom admin theme to inherit properties from the default theme. 4. Removes the sidebar text color override for mobile. 5. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 6. Restores the appBar hiding functionality for desktop/large screens. 7. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. The "show production hubs logo in the storybook..." commit broke the display of the image in deployed instances. 3. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 4. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 5. It looks better to have the sidebar be a single consistent color from top to bottom. 6. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 7. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization
What: 1. Removes much of the AI styling changes. 2. Reverts a21f8d4 (show production hubs logo in the storybook...). 3. Changes the custom admin theme to inherit properties from the default theme. 4. Removes the sidebar text color override for mobile. 5. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 6. Restores the appBar hiding functionality for desktop/large screens. 7. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. The "show production hubs logo in the storybook..." commit broke the display of the image in deployed instances. 3. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 4. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 5. It looks better to have the sidebar be a single consistent color from top to bottom. 6. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 7. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization
…rom production scenario so storybook is a closer match. Decompose chrome into a separate file.
|
From @matthargett in Discord:
|
Exairnous
left a comment
There was a problem hiding this comment.
@matthargett
This is mostly good now. Thank you for the more robust scrolling solution, that's a very nice improvement! And thank you for all the other work you've done on this, it's going to be great to have all of this updated and working!
There are a few small things, though.
the adminTheme in admin/.storybook/preview.js seems to be a partially duplicated version of the one in admin/src/admin.js with a few tweaks, although the visuals look mostly the same, which is good. Would it be possible to import the theme from admin/src/admin.js, so that we don't have duplicate code that needs to stay synced? The Storybook preview also seems to be missing the main background color change for the sidebar that prevents flashing when scrolling and white from showing up at the bottom underneath the scrollable area (if that can be imported as well, that would further reduce maintenance in the future).
I get errors when attempting to view the InvitePopover stories and the Tip stories (screenshots below). @DougReeder reported in Discord that they work fine for him, however I was able to fix the InvitePopover stories by adding shareUrlHandler={shareInviteUrl.bind(this, useIntl(), room.url, {roomName: "room-name", appName: "app-name"})} to the relevant spots (along with the needed imports at the top), so I guess this is a browser (Brave) or operating system (Arch Linux) issue.
I wasn't able to find a fix for the Tip stories, but it would be nice to get them fixed for me too, if possible.
InvitePopover error screenshot:

The SpectatingLabel story just shows a blank screen for me.
The rest of my thoughts have been left as inline comments.
matthargett
left a comment
There was a problem hiding this comment.
I have no idea why github put my replies to comments on my own PR as a review mode thing
…ght viewport with a dark backdrop, matching how the app overlays it so the story no longer appears blmak.
What: 1. Removes much of the AI styling changes. 2. Reverts a21f8d4 (show production hubs logo in the storybook...). 3. Changes the custom admin theme to inherit properties from the default theme. 4. Removes the sidebar text color override for mobile. 5. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 6. Restores the appBar hiding functionality for desktop/large screens. 7. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. The "show production hubs logo in the storybook..." commit broke the display of the image in deployed instances. 3. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 4. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 5. It looks better to have the sidebar be a single consistent color from top to bottom. 6. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 7. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization
I tried it in Firefox and everything seems to work fine, so I guess it was just something to do with Brave (or my version of it). Thank you (and @DougReeder) for the extra testing and the screenshots. |
There was a problem hiding this comment.
Thank you for the update! Aside from what I've mentioned in replies to the inline messages, there are a couple other small things I found.
- It looks like you forgot to upload the admin-theme file.
- The
storybook-staticfolders for both hubs and admin should probably be added to.gitignore?
|
Scrolling in the Admin Panel works, and I'm not seeing any regressions in Hubs Storybook. Admin Storybook worked in commit ad1ac24 but in the latest it fails with "Module not found: Error: Can't resolve '../src/admin-theme' in '/Users/doug/git/hubs/admin/.storybook'" |
…that runs automated tests. Why: There is no native way to include Git hooks in the checked-in code. All solutions require either running a command or installing a 3rd party package. As Hubs developers will almost always run `npm install` after cloning the repo, this approach requires no change in development steps. Note that developers can always prevent a hook from running by adding the flag `-n`, so git hooks just support fail-fast. Enforcement has to occur in CI.
…missing admin-theme shared file.
Exairnous
left a comment
There was a problem hiding this comment.
Thanks. This is looking mostly good now. I think once the null guard discussion is finished we can merge (assuming @DougReeder hasn't found any other issues).
Co-authored-by: Exairnous <mythologylover75@gmail.com>
|
The latest code builds and runs on my instance. |
What: 1. Removes much of the AI styling changes. 2. Reverts a21f8d4 (show production hubs logo in the storybook...). 3. Changes the custom admin theme to inherit properties from the default theme. 4. Removes the sidebar text color override for mobile. 5. Fixes the issue with the sliding section of the light theme color at the top of the sidebar and ensures the dark theme color extends to the bottom of the sidebar (regardless of browser zoom). 6. Restores the appBar hiding functionality for desktop/large screens. 7. Fixes the issues with the sidebar scrolling by removing the scrolling wrapper and instead overriding the default react-admin sidebar with a custom version that has scrolling indicators that display whenever there is hidden content (either above or below the shown content). Why: 1. The AI styling changes seem to be unnecessary and to have contributed to the visual inconsistencies/glitches. 2. The "show production hubs logo in the storybook..." commit broke the display of the image in deployed instances. 3. Custom themes inheriting properties from the default theme is recommended in the React Admin documentation[1]. 4. The sidebar text color override for mobile is no longer needed (and causes readability issues) now that the background color is the same for both mobile and desktop. 5. It looks better to have the sidebar be a single consistent color from top to bottom. 6. This keeps the behavior of the appBar the same as it was before the Update storybook PR (Hubs-Foundation#6563) 7. There were two scrolling areas, which caused the scrolling to appear erratic, and the scrolling indicators weren't updated live (live updates ensures that people are always aware when the content can be scrolled and in which direction). [1] https://marmelab.com/react-admin/doc/3.19/Theming.html#sidebar-customization










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
Claude Code and OpenAI Codex were used to assist on this pull request.