Need a react component wrapping this api

[phenax]

A clean React component <PatternLock />

[yj7o5]

@phenax I am trying to get started with some open source, can I start contributing ... I thought this will be the right project to start with small, minimal.
Few questions what exactly are you envisioning besides just a standalone Component, like any other functionality to be added to this ...?

[phenax]

@yj7o5, Awesome!
A basic wrapper simplifying the config to sensible props would do.
One heads-up before you start tho. Re-renders! Be careful about how you handle the changing props.

If you have any further questions about the project, feel free!

[yj7o5]

cool, will get repo. cloned and get started ... wish me good luck and that perseverance keep me to contribute to the open source :)

[yj7o5]

Ok before I really go into the implementation here are some of my thoughts:-

1. Do we want to maintain the React "paradigm" throughout that "immutability" or
2. We are good with having some sort of mutability
   and heres why in the react world we would want to have as much immutability as possible and that gets achieved when we are utilizing the props and having the state that drives the UI.
   The way you have it is direct manipulation of the canvas element which makes sense as that how the Canvas api works you directly call the canvas API for the change to happen. Now translating it over in the React world would mean exposing/expecting props or having a HOC(higher order component) that takes in Props that drives behavior.
   Then this comes to mind this lib I found from flipboard.com https://github.com/Flipboard/react-canvas
   we can take this route so that we remain close to the principle.
   Or we could be a little lenient and just render the Canvas element first and directly manipulate the element.
   I personally like the formal approach (though it maybe a little more work than just expecting certain props) as we have added additional layer of abstraction but I think it will be a bit more flexible and be close to the react principle.

Any thoughts? What do you think ...

[phenax]

@yj7o5,
The initial idea and the title for this issue was to have an react api to wrap the existing library. That being said, the thought of rewriting the entire library in react did cross my mind before but the problem with that approach is that then it no longer remains a no dependency, plug and play kinda deal. If you are thinking of a wrapper that stores everything in the state and then on an change in state, updating the state of the internal library then I say go for it. If you feel like it, you can create a patch method in the internal library (a generic one) to plug the updates through.
So in summary, I am all for refactoring the library to have the updates be more declarative but it is really important for the existing api to be lightweight dependency-wise. A setup like either @phenax/pattern-lock-js for vanilla and @phenax/pattern-lock-js/react to get the react component or the full yarn workspaces deal with a monorepo.

packages/pattern-lock-js-core
packages/pattern-lock-js-react
packages/pattern-lock-js-common

[yj7o5]

btw I am getting error when pushing the branch to remote:-

fatal: unable to access 'https://github.com/phenax/pattern-lock-js.git/': The requested URL returned error: 403
> git push -u origin react-api-v1
remote: Permission to phenax/pattern-lock-js.git denied to yj7o5.
fatal: unable to access 'https://github.com/phenax/pattern-lock-js.git/': The requested URL returned error: 403

Let me know if you need more logs ...
PS: Do you think maybe because I am on node v8 and npm v6.4?

[yj7o5]

ok for the first pass, I'll keep it light weight and just provide a component.
Once that looks solid enough then we can move onto being a bit more declarative such as(and explore diff. venues such as providing core/react/common grounds):

<PatternLock>
<Ring />
<Ring />
<Ring />
</PatternLock>

etc.

Yea I was a bit iffy too on whether to add yet another lib. as a dependency.

[phenax]

@yj7o5, You should create a fork of the repo and then push to a branch there. When you're done or you want something reviewed, you can create a pull request on this repo. When the pull request reaches a merge-able state, we'll merge it to master.

About the api that you are proposing, it looks good but I don't see a point in the user of the api having to add 9 child Ring elements instead of passing props like rows={3} cols={3}. I'm asking this since there's no way to customise individual ring.

You can have a wrapper for the match api. This will turn the pattern green if its a match and red if it's not and pass in a handler for onSuccess and onFailure.

<PatternLock {...patternProps}>
    <Match hash={'hgA223jj2g22=='} />
    <Match hash={'MAA22j52g22=='} />
</PatternLock>

[yj7o5]

hmm ok yea forgot that forking part. Anyways I feel like I now have gotten a plan to work on, will start the work and keep you updated.