Conversation
packages/pluggableWidgets/signature-web/src/__tests__/AppEvents.spec.tsx
Outdated
Show resolved
Hide resolved
packages/pluggableWidgets/signature-web/src/components/Alert.tsx
Outdated
Show resolved
Hide resolved
packages/pluggableWidgets/signature-web/src/assets/Signature.icon.dark.svg
Show resolved
Hide resolved
packages/pluggableWidgets/signature-web/src/ui/SignaturePreview.css
Outdated
Show resolved
Hide resolved
packages/pluggableWidgets/signature-web/src/assets/Signature.icon.dark.png
Outdated
Show resolved
Hide resolved
packages/pluggableWidgets/signature-web/src/Signature.editorConfig.ts
Outdated
Show resolved
Hide resolved
gjulivan
force-pushed
the
signature-web
branch
8 times, most recently
from
1edd154 to
c439717
Compare
| if (imageDataUrl) { | ||
| imageSource.setValue(Utils.convertUrlToBlob(imageDataUrl)); | ||
| } | ||
|
|
||
| // Trigger microflow to update signature attribute | ||
| if (onSignEndAction) { | ||
| onSignEndAction(imageDataUrl); | ||
| } |
There was a problem hiding this comment.
Looking at the code, maybe it makes more sense to handle everything in onSignEndAction instead of scattering business logic between two places (here and wherever onSignEndAction is defined).
| @@ -0,0 +1,3 @@ | |||
| <!-- TODO: Update marketplace URL --> | |||
|
|
|||
| Please see [App Events](https://docs.mendix.com/appstore/widgets/) in the Mendix documentation for details. | |||
| "@mendix/widget-plugin-hooks": "workspace:*", | ||
| "@mendix/widget-plugin-platform": "workspace:*", | ||
| "@mendix/widget-plugin-test-utils": "workspace:*", | ||
| "cross-env": "^7.0.3" |
There was a problem hiding this comment.
Looks like we don't use this
| }, | ||
| "packagePath": "com.mendix.widget.web", | ||
| "marketplace": { | ||
| "minimumMXVersion": "10.8.0", |
| <caption>Height unit</caption> | ||
| <description /> | ||
| <enumerationValues> | ||
| <enumerationValue key="percentageOfWidth">Auto</enumerationValue> |
There was a problem hiding this comment.
Maybe it is better to name the key auto as well.
| onSignEndAction?: (imageUrl?: string) => void; | ||
| wrapperStyle?: object; | ||
| readOnly: boolean; | ||
| } |
There was a problem hiding this comment.
It is kind of confusing that is removes property and then reads it with the same name onSignEndAction and different signature. Also some properties are redefined as is, like readOnly or showGrid. I believe, if we need to change the properties shape we better pre-process them and give them a meaningful shape before passing to the underlying components. For example we can define in types that if we don't need to show grid, grid* properties are not defined. See Barcode generator for a good example.
There was a problem hiding this comment.
We probably can use useResizeObserver from our common utils. We didn't do this before because it was a custom widget, not anymore.
| console.log( | ||
| "Try Initializing signature pad", | ||
| penColor, | ||
| signaturePadOptions, | ||
| canvasRef.current, | ||
| signaturePadRef.current, | ||
| imageSource, | ||
| hasSignatureAttribute | ||
| ); |
There was a problem hiding this comment.
parseStyle in this file is not being used
| <SignatureComponent | ||
| readOnly={false} | ||
| {...props} | ||
| className={className} | ||
| onSignEndAction={handleSignEnd} | ||
| clearSignature={false} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Why readOnly and clearSignature are hardcoded?
2 participants
Pull request type
Description