(refactor): Wrap upstream signature_pad #20
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Apparently react-signature-pad (what this repo forked from) just
copy+pasted most of an older version of signature_pad when it was
first created
- In order to get all the latest features, fix bugs, and reduce the
maintenance burden an general surface of this library and its API,
react-signature-canvas is now a React wrapper around signature_pad
- It retains the features it added on top of signature_pad, such as
automatic resizing, a trim feature, and idiomatic React updates
via props
- And now has new features like fromDataURL options, toDataURL,
fromData, and toData! And new props like minDistance and throttle
- Return SignaturePad instance via getSignaturePad
- Use componentDidMount to trigger prop updates via Object.assign on
the SignaturePad instance
- Move all wrapper methods to below render for better organization
- Update README with new props and API methods
- Add links to signature_pad in the props and API sections
- Change intro to reflect the new changes in this library / repo
- Remove bezier.js and point.js as they are wrapped
- Remove all mouse / click / touch handling and other core calculations
that are now wrapped
- Remove defaultProps that were for the signature_pad implementation
- This also once again changes up how the dotSize function works
- It is now back to how it worked pre-v0.2.0
- Still debatable if this is a bugfix or a breaking change
(see v0.2.2)
- Left in propTypes for signature_pad, but maybe those should be
removed too?
- also update keywords
|
Oh man, great cleanup job. 👍 |
|
Maybe I'm doing something terribly wrong but I couldn't run the example without adding the |
- perhaps should actually contribute to upstream and have szimek
change it to the new implementation
- (minWidth, maxWidth) => (minWidth + maxWidth) / 2 is far more
intuitive than having to bind to the right this context
|
@lopis tbh, I hadn't ran the example in almost 2 years (since I made the fork haha), but ran Not sure if there might be a dependency conflict with your install (since the webpack and babel versions used here are out-of-date nowadays) |
|
If it works on your side it must be some dependency on my side. If I find
something I'll let you know.
…On Mon, 2 Apr 2018, 00:11 Anton Gilgur, ***@***.***> wrote:
@lopis <https://github.com/lopis> tbh, I hadn't ran the example in almost
2 years (since I made the fork haha), but ran npm start in my local copy
and it worked no problem o.o. The react preset is specified in
webpack.config.js so that a .babelrc isn't necessary.
Not sure if there might be a dependency conflict with your install (since
the webpack and babel versions used here are out-of-date nowadays)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AClwZ_8Lb5m67mZKU68-vq3xf0cZ9Ak_ks5tkVCGgaJpZM4TCsyW>
.
|
|
If this looks good, I'll release the alpha version tonight/tomorrow |
|
Rebased in as a703e22 and 62c3714 and released as v1.0.0-alpha.1. Thanks for the review @lopis ! If anyone can sees this can test the alpha within their own codebase, would be helpful and appreciated! |
This was referenced Apr 12, 2018
Closed
Repository owner
locked as resolved and limited conversation to collaborators
Mar 13, 2021
agilgur5
added
the
scope: dependencies
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #17