Convert to functional style and expose more types for the upcoming version 3 release#101
Merged
Convert to functional style and expose more types for the upcoming version 3 release#101
Conversation
jamesrweb
added
enhancement
dependencies
jamesrweb
force-pushed
the
refactor/v3
branch
5 times, most recently
from
65a4c4c to
765dec4
Compare
jamesrweb
changed the title
yevdyko
approved these changes
Jun 26, 2021
Contributor
yevdyko
left a comment
yevdyko
left a comment
There was a problem hiding this comment.
Outstanding work!
It would be nice if you could explain in the description the difference between v2 and v3, why you introduced these changes and what problems do they solve?
| "p5": "^1.3.1" | ||
| }, | ||
| "peerDependencies": { | ||
| "react": "*", |
Contributor
There was a problem hiding this comment.
I'm wondering if you want to define the React version here, based on the fact that you use React hooks in your implementation. For example, 16.8 is the version that supports them.
| attributes: TAttributes; | ||
| canvas: p5; | ||
| wrapper?: HTMLElement; | ||
| export interface P5WrapperProps { |
Contributor
There was a problem hiding this comment.
After changing the component name to ReactP5Wrapper should it also be applied here? ReactP5WrapperProps?
| const wrapper = createRef<HTMLDivElement>(); | ||
| const [instance, setInstance] = useState<P5Instance>(); | ||
|
|
||
| useEffect(() => { |
Contributor
|
Auto-merge was not the behaviour I expected 😕 |
Closed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Closes #90.
Closes #87.
This PR will, when completed, implement a new functional react version of the wrapper instead of the old class based component. The docs need updated, some build configurations need changed to work with
rollupas we move away fromwebpackbut overall the implementation is nearly complete!