Skip to content
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

add change handling for props #146

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add change handling for props #146

wants to merge 1 commit into from

Conversation

andyrichardson
Copy link

  • Updates event handlers on prop change
  • Removes switch logic for lookup map
  • Removes event handlers on unmount

Fix #142

@coveralls
Copy link

Coverage Status

Coverage increased (+15.9%) to 97.059% when pulling c28117d on andyrichardson:fix-prop-change-handler into a633e7d on scniro:master.

@scniro
Copy link
Owner

scniro commented Jan 19, 2020

@andyrichardson I am a lot shorter on time these days as when I started this project. Codemirror & React APIs are moving to quickly for me to keep atop of for the day-to-day. I am looking for a co-maintainer of this project. Please contact me directly if you are interested. Thank you for understanding.

import * as React from 'react';
import * as codemirror from 'codemirror';
import {Position} from "codemirror";
import * as React from "react";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @andyrichardson - I'm going to help to get this merged if you're still up for it?
Soo... I don't see anything in the .editorconfig file that defines the spacing / quotes style for this project but your change introduces a number of purely stylistic changes that I think are ancillary. I personally like the style you're using but I don't want to mix/match. Would you be up for either a) removing stylistic changes b) committing changes to .editorconfig to either cause your style or the original one to be applied throughout?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @elsigh I'm more than happy to go with whatever stylistic preferences the project has! Sorry about the unnecessary changes, I have prettier on by default which goes HAM on everything.

Copy link
Collaborator

@elsigh elsigh May 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andyrichardson - I had the same issue - I had to use Cmd-Shift-P to save without formatting.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gentle ping @andyrichardson =)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will sort this out this week!

I hope... 😉

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would still love to merge this one if you get a little time

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to see this merged still if possible! @andyrichardson

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

Successfully merging this pull request may close these issues.

Component prop updates are ignored
5 participants