-
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
Bugfix: Don't throw error in FormPreview if assetDatabase is not a UUID #9531
Bugfix: Don't throw error in FormPreview if assetDatabase is not a UUID #9531
Conversation
@@ -45,7 +45,7 @@ const RichTextWidget: React.FunctionComponent<WidgetProps> = ({ | |||
onBlur(id, editor.getHTML()); | |||
}} | |||
editable={!(disabled || readonly)} | |||
assetDatabaseId={validateUUID(database)} | |||
assetDatabaseId={typeof database === "string" && isUUID(database) ? database : null} |
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.
Leave a comment that the database will be an object in the preview when a variable is provided?
@@ -29,7 +29,7 @@ import ErrorToast from "@/components/richTextEditor/ErrorToast"; | |||
|
|||
type EditorProps = EditorProviderProps & { | |||
// A PixieBrix asset database ID to use for uploading images. If not included, the image extension will be disabled. | |||
assetDatabaseId?: UUID; | |||
assetDatabaseId?: UUID | null; |
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.
Is there a reason to need both ?
and | null
? You could just pass undefined instead of null?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9531 +/- ##
==========================================
+ Coverage 74.24% 75.81% +1.56%
==========================================
Files 1332 1429 +97
Lines 40817 43156 +2339
Branches 7634 7938 +304
==========================================
+ Hits 30306 32719 +2413
+ Misses 10511 10437 -74 ☔ View full report in Codecov by Sentry. |
Playwright test resultsDetails Open report ↗︎ Flaky testsmsedge-setup › setup/unaffiliated.setup.ts › authenticate with unaffiliated user Skipped testschrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options |
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.
See #9531 (comment)
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.
See comments on readability
No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack. Do not edit this comment manually. |
Merging the PR to get everything in main before we switch over to pixiebrix-source. Please follow up comments in the new repo. |
What does this PR do?
Fixes this 👇