Closed colbyfayock closed 3 months ago
I'm working on implementing these methods and thinking about the direction of generateUploadWidgetResultCallback
In my example, Next.js, I'm using the callback methods in 2 ways, 1 as a direct callback (onSuccess
) and as a supplemental one as an Action (onSuccessAction
) with a slightly different signature
here's an initial look at the implementation of this function along with the previous logic that split up these different callbacks
const resultsCallback = generateUploadWidgetResultCallback({
onError: (uploadError) => {
setError(uploadError);
if ( typeof onError === 'function' ) {
onError(uploadError, {
widget: widget.current,
...instanceMethods
});
}
},
onResult: (uploadResult) => {
if ( typeof uploadResult?.event !== 'string' ) return;
setResults(uploadResult);
const widgetEvent = WIDGET_EVENTS[uploadResult.event] as keyof typeof props;
if ( typeof widgetEvent === 'string' && typeof props[widgetEvent] === 'function' ) {
const callback = props[widgetEvent] as CldUploadEventCallback;
callback(uploadResult, {
widget: widget.current,
...instanceMethods
});
}
const widgetEventAction = `${widgetEvent}Action` as keyof typeof props;
if ( widgetEventAction && typeof props[widgetEventAction] === 'function' ) {
const action = props[widgetEventAction] as CldUploadEventAction;
action(uploadResult);
}
}
});
this somewhat deters from the original simplicity goal where this requires still defining the callbacks and handling them all within a single onResult
callback as opposed to just dumping the callbacks into the widget, but it feels like this may be unavoidable in this instance and any code to try to supply both callbacks is ultimately going to result in a bunch of code on the Next.js side
the other point is each of the callbacks in the generateUploadWidgetResultCallback
are only Typed to pass the the results of the event, not the results and the instance methods as is currently shown here. this is also somewhat unavoidable because this is intentionally detatched from instance creation so it never knows about it, leaving some of this code additionally unavoidable
does this method even help? if only a little bit? it may provide a slightly cleaner interface and centralize some, but not all of the props
would love your thoughts @Baroshem and @ghostdevv
I think it's ok to have a onResult
callback that is then up to us to handle because each framework has different ways of doing events that might be otherwise incompatible
:tada: This PR is included in version @cloudinary-util/util-v3.2.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
:tada: This PR is included in version @cloudinary-util/url-loader-v5.6.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
:tada: This PR is included in version @cloudinary-util/types-v1.2.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket:
This creates a new set of helpers that will help framework libraries reduce the boilerplate needed to instantiate new instances of the upload widget.
Issue Ticket Number
Fixes #164
Usage
Description
Issue Ticket Number
Fixes #
Type of change
Checklist