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

ReagentInput `:ref` broken #259

Closed
Deraen opened this Issue Sep 13, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@Deraen
Member

Deraen commented Sep 13, 2016

42ba166 broke using :ref attribute with input elements. This is because :ref attribute is always overriden for these elements:

($! :ref #($! this :cljsInputElement %1))))))

If the code keeps on going using :ref callback, it must keep track of the original function and call that. If ref callback is used, string ref can never be supported here, but maybe that is not a problem as those haven't really worked with Reagent anyway.

@holmsand

This comment has been minimized.

Show comment
Hide comment
@holmsand

holmsand Sep 14, 2016

Contributor

Great catch! The patch looks good too as far as I can tell – I'll put that into a 0.6.1. Thanks!

Contributor

holmsand commented Sep 14, 2016

Great catch! The patch looks good too as far as I can tell – I'll put that into a 0.6.1. Thanks!

@pesterhazy

This comment has been minimized.

Show comment
Hide comment
@pesterhazy

pesterhazy Sep 15, 2016

Contributor

FWIW I do use string based :refs for input elements. Mostly for historical reasons -- ref callbacks (which are superior) didn't exist a while ago.

Contributor

pesterhazy commented Sep 15, 2016

FWIW I do use string based :refs for input elements. Mostly for historical reasons -- ref callbacks (which are superior) didn't exist a while ago.

@pesterhazy

This comment has been minimized.

Show comment
Hide comment
@pesterhazy

pesterhazy Sep 15, 2016

Contributor

BTW I use (aget (.-refs (r/current-component)) "my-input") to get the backing instance of the component

Contributor

pesterhazy commented Sep 15, 2016

BTW I use (aget (.-refs (r/current-component)) "my-input") to get the backing instance of the component

@Deraen

This comment has been minimized.

Show comment
Hide comment
@Deraen

Deraen Sep 15, 2016

Member

Check PR #260, looks like ReagentInput will be using findDOMNode so it doesn't need to touch refs at all.

Member

Deraen commented Sep 15, 2016

Check PR #260, looks like ReagentInput will be using findDOMNode so it doesn't need to touch refs at all.

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