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

Remove fix once it is fixed in std:accounts-ui #4

Open
PolGuixe opened this issue Sep 7, 2016 · 10 comments
Open

Remove fix once it is fixed in std:accounts-ui #4

PolGuixe opened this issue Sep 7, 2016 · 10 comments
Assignees

Comments

@PolGuixe
Copy link
Contributor

PolGuixe commented Sep 7, 2016

Remove fix.js once std:accounts-ui is fixed.

studiointeract/accounts-ui#60

@PolGuixe
Copy link
Contributor Author

fixed and removed fix.js

@PolGuixe
Copy link
Contributor Author

PolGuixe commented Nov 15, 2016

Added fix again. It seems it is not completely fixed in std:accounts-ui

Need to investigate more.

@PolGuixe PolGuixe reopened this Nov 15, 2016
@PolGuixe
Copy link
Contributor Author

PolGuixe commented Jan 11, 2017

@timbrandin studiointeract/accounts-ui#60 has been fixed but I am still getting this issue.

I still need https://github.com/Zetoff/accounts-material-ui/develop/fix.js otherwise I get this error:

LoginForm.jsx:204 Uncaught TypeError: Cannot read property 'trim' of undefined
    at LoginForm.handleChange (LoginForm.jsx:204)
    at Field.triggerUpdate (Field.jsx:16)
    at Field.componentDidMount (Field.jsx:21)
    at modules.js?hash=e87bb51…:187268
    at measureLifeCyclePerf (modules.js?hash=e87bb51…:187078)
    at modules.js?hash=e87bb51…:187267
    at CallbackQueue.notifyAll (modules.js?hash=e87bb51…:180089)
    at ReactReconcileTransaction.close (modules.js?hash=e87bb51…:190339)
    at ReactReconcileTransaction.closeAll (modules.js?hash=e87bb51…:181248)
    at ReactReconcileTransaction.perform (modules.js?hash=e87bb51…:181195)

Any idea?

@dschreij
Copy link

dschreij commented Jan 18, 2017

The fix causes another error for me: studiointeract/accounts-ui#92. Apparently, the input object hasn't even been created yet in some cases, causing the reference to value to throw an exception.

@dschreij
Copy link

dschreij commented Jan 18, 2017

Maybe we can add an extra check to see if 'input' exists?

triggerUpdate() {
		const {onChange} = this.props;
                let value;
		if(this.input) value = this.input.value;

		if (value === undefined) {
			value = '';
		} else {
			// do nothing
		}

		if (this.input) {
			onChange({target: {
					value
				}})
		}
	}

This could be done even prettier of course, but I was trying to change as little as possible.

@PolGuixe
Copy link
Contributor Author

@dschreij I agree that it can be done even prettier. It's good for now. In the mid-term - I need to find some time to focus and nail this - I would love to completely remove this fix.

I've tested the your proposed fix and and works great. I'll pushed now.

Next time feel free to just do a PR to develop ;)

PolGuixe added a commit that referenced this issue Jan 19, 2017
PolGuixe added a commit that referenced this issue Jan 19, 2017
@PolGuixe
Copy link
Contributor Author

@dschreij please let me know if the current version works for you.

@dschreij
Copy link

@PolGuixe sorry, I might indeed as well have sent you a PR. I am using your atmosphere package now. Can I just update it there or do I need to use this package from Github to test it (I need to first find out then how to add packages from Github in meteor). Thanks for responding this quickly!

@PolGuixe
Copy link
Contributor Author

PolGuixe commented Jan 19, 2017

@dschreij the package is now on atmosphere ;) just do meteor update zetoff:accounts-material-ui and it should update to v0.0.11.

@dschreij
Copy link

Yes! It seems fixed now. Thanks a lot!

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

No branches or pull requests

2 participants