-
Notifications
You must be signed in to change notification settings - Fork 22
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
Fixes brick position reset and simplifies form reinitialization #9351
Changes from all commits
3a208cd
25e4719
d52781a
f97e8a3
7b33ff4
f84e91c
5475120
9530a8c
ec247ac
0957e74
a80ab97
251b6d6
f1f7812
05f725e
70ad346
255dd84
651ad4c
88923e9
213ff26
0aa7458
7cf322f
5b1de6a
80e82fc
91cfb5b
ec09a3c
67fb4db
ce153b1
6776dcf
e5fd675
6a2e9ce
dbe66be
383e0f5
226123c
3a39592
00f703f
8aec8a9
2a39b66
17ad8e1
06817f0
c4f0385
2fcfb70
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,7 +51,9 @@ import { useDispatch } from "react-redux"; | |
import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"; | ||
|
||
/** | ||
* Handles possible NPE from `config.value` being undefined. | ||
* Handles possible NPE from the brick configuration not being migrated. | ||
* Ensures that the brickPipelineUIState is valid | ||
* | ||
* This originally happened during the extension -> modComponent form state migration. | ||
* See https://github.com/pixiebrix/pixiebrix-extension/issues/8781 | ||
*/ | ||
|
@@ -60,7 +62,12 @@ function useRecoverFormStateIntegrityError(name: string) { | |
const [config] = useField<BrickConfig | undefined>(name); | ||
const dispatch = useDispatch(); | ||
|
||
if (!config.value) { | ||
/** | ||
* If the name doesn't include "modComponent", the brickPipelineUIstate has not been migrated | ||
* and is invalid. Previously, we checked for the existence of config.value, but config.value can be undefined | ||
* (see usage in BrickConfiguration). Remounting the form instead of reinitializing hid that logic error. | ||
*/ | ||
if (!name.includes("modComponent")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before merging: documentation for this func needs to be updated if we're removing the check for Non-blocking as I trust you on form state migration: but can you help me understand how this is equivalent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree see outdated comment: #9351 (comment) I have very limited understanding of the context behind that hook. I did rename it recently though to be more clear on why it exists (it used to just be called useReset (or a similar name) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improved the comment. Let me know if there's still confusion |
||
dispatch( | ||
editorActions.setModComponentFormState({ | ||
modComponentFormState: context.values, | ||
|
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.
Functional changes are all here