Skip to content

Hot Refresh Support #1148

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

Merged

Conversation

ryanzhangofficial
Copy link
Member

Refactors ThemeContext, ModalContext, and PanelContext to follow React Fast Refresh guidelines by exporting only components/hooks. Moves helpers to separate .ts files to avoid full reloads on UI changes. More info in docs.

@BrandonPacewic BrandonPacewic added the ui/ux Relating to user interface, or in general, user experience label Jun 20, 2025
Copy link
Member

@BrandonPacewic BrandonPacewic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Love that this persistent issue is finally being fixed, we left it here for far to long imo.

Two small things:

  1. We use pascal case for files so useModalManager.tsx should be UseModalManager.tsx.
  2. Small merge conflicts with dev.

@ryanzhangofficial ryanzhangofficial force-pushed the ryan/1706/hot-refresh-support branch from 219ff55 to 8e856d4 Compare June 20, 2025 19:08
@ryanzhangofficial
Copy link
Member Author

Noted! Updated the file nomenclature. I tried to merge the conflicts but the new Jolt module seems to be giving me problems.

Copy link
Member

@BrandonPacewic BrandonPacewic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted! Updated the file nomenclature. I tried to merge the conflicts but the new Jolt module seems to be giving me problems.

Make sure to run npm install again (or whatever package manager you use). If your still having issues let me know specifics and I can help resolve them.

Copy link
Member

@BrandonPacewic BrandonPacewic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryanzhangofficial
Copy link
Member Author

Thanks for the useTheme catch. ThemeProvider, however, should stay because it's the React component. I realized that the useModelControlContext and usePanelControlContext hooks were both still in ModelContext and PanelContext and did some further component isolation.

Copy link
Member

@BrandonPacewic BrandonPacewic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the useTheme catch. ThemeProvider, however, should stay because it's the React component. I realized that the useModelControlContext and usePanelControlContext hooks were both still in ModelContext and PanelContext and did some further component isolation.

Seems reasonable, just got to make sure the workflows pass and we can get this in.

Copy link
Member

@PepperLola PepperLola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked perfectly, I was able to edit the main hud and have it hot reload without any errors or unintended behavior.

Copy link
Collaborator

@Dhruv-0-Arora Dhruv-0-Arora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing

@PepperLola PepperLola dismissed BrandonPacewic’s stale review June 23, 2025 18:06

Workflows pass now

@PepperLola PepperLola merged commit 86b94d3 into Autodesk:dev Jun 23, 2025
15 checks passed
@BrandonPacewic BrandonPacewic mentioned this pull request Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/ux Relating to user interface, or in general, user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants