-
Notifications
You must be signed in to change notification settings - Fork 47
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
fix: MenuButtonImageUpload should call insertImages prop #188
Conversation
ec7b064
to
692b16f
Compare
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.
Great catch, thanks for opening the PR @lorenries! Just one minor suggestion, which should simplify things a bit and make it more consistent with some other similar code elsewhere.
@@ -52,6 +55,7 @@ export interface MenuButtonImageUploadProps | |||
export default function MenuButtonImageUpload({ | |||
onUploadFiles, | |||
inputProps, | |||
insertImages, |
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.
For consistency with a similar use-case here:
mui-tiptap/src/controls/ColorPicker.tsx
Line 131 in 96a1cae
colorToHex = colorToHexDefault, |
how about:
insertImages, | |
insertImages = insertImagesDefault, |
(importing with insertImages as insertImagesDefault
instead of insertImages as fallbackInsertImages
).
And then no need to update any of the code below (no conditional needed)
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.
A refactored version of #188.
A refactored version of #188.
Thanks again for catching this bug and suggesting the fix! Since the final change is so small (with the refactor I suggested a few months back), I've gone ahead and made a separate smaller commit in a branch here #214. Closing this out. |
The
MenuButtonImageUpload
component doesn't actually call theinsertImages
prop, although it's defined in the type signature.