Skip to content

Circle support #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 7, 2018
Merged

Circle support #258

merged 6 commits into from
Oct 7, 2018

Conversation

ryanoboril
Copy link

I've tried a few of the existing pull requests for this functionality and they did not work for me. I have re-added the functionality myself in the hopes that this can be merged. I have included a screenshot and some example code in the README. Let me know what else needs to be done to get this in.

@miliu99
Copy link

miliu99 commented Oct 5, 2018

This Circle component works, however, it appears it's missing the componentWillReceiveProps event handling, so that when center changes it doesn't redraw. I have to refresh the browser to make it redraw.

@ryanoboril
Copy link
Author

The circle is now re-rendered on any prop change among props defined in Circle.propTypes. Also the options are now pulled up to prop level, so they can now trigger rerendering as well, no need to pass nested options object (which will not trigger a re-render on change).

@miliu99
Copy link

miliu99 commented Oct 6, 2018

This works great. Thank you very much! Why not release it?

@ryanoboril
Copy link
Author

I don't have merge access for this repo, so all I can do is submit a pull request and wait for it to be reviewed.

@auser Is anyone available to review this?

@auser auser merged commit 85031c5 into fullstackreact:master Oct 7, 2018
@auser
Copy link
Contributor

auser commented Oct 7, 2018

Thanks! It’s perfect

@ryanoboril
Copy link
Author

No problem, thanks for making this module. It's great!

@miliu99
Copy link

miliu99 commented Oct 8, 2018 via email

@ryanoboril
Copy link
Author

@auser Circle does not appear in dist, was there a successful prod rebuild prior to npm publish? If i pull the repo into my node_modules it works fine, but with the 2.0.2 npm there is no transpiled dist/components/Circle.js file and it looks like the transpiled index.js in dist does not export it. It doesn't look like the latest.

@miliu99
Copy link

miliu99 commented Dec 5, 2018

It appears the circles in 2.0.2 is working now.

@miliu99
Copy link

miliu99 commented Dec 5, 2018

Sorry, I spoke a little too early...it is strange that it works fine in dev mode, but not in production mode - I got "Minified react error #130".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants