search

Bernie-2016 / ground-control

Central volunteer and data portal
https://organize.berniesanders.com
GNU Affero General Public License v3.0
63 stars 25 forks source link

Fix text editor cursor placement #514

Closed jwarnow closed 8 years ago

jwarnow commented 8 years ago

Text editor acts weird – can only add text at end of message: http://cl.ly/3V413w2W3g01

jwarnow commented 8 years ago

I tested more -- you can add just one character mid-message, and then the cursor jumps back down to the bottom.

ejucovy commented 8 years ago

Hm - looks like this bug exists in other parts of the codebase too, like the event edit forms (any text input or textarea that isn't WYSIWYG-ified)

I submitted a PR #540 that fixes it in the FastFwd interface, but if that looks good then I can try to extend it to other pages too.

saikat commented 8 years ago

https://github.com/jquense/react-formal/issues/33 is the underlying issue . It should be fixed with new react-formal release.

bahoo commented 8 years ago

This is now live.

jlegrone commented 8 years ago

This seems to have cropped up again, and we are on the latest react-formal release (0.15.6).

PhearZero commented 8 years ago

PR #624 should resolve the issue with all the forms I could find that where not binding the form value properly. Should fix this issue

bahoo commented 8 years ago

This is up. Thanks again, @PhearZero!

jlegrone commented 8 years ago

Still an issue on stock material-ui text inputs.

jlegrone commented 8 years ago

Just verified that this was introduced in material-ui 0.15.x.

PhearZero commented 8 years ago

@jlegrone Do you have an issue link that I can look at? The Forms did not have a "model" bind required for react-formal. The fields that do not belong to a form shouldn't bind to value but instead set up a valueLink or wrap it in a Form component. Anyway the last of the forms are patched and the last field is the GCPhoneField. I can patch it but I don't have a good way to handle the formatting. Alpha Material TextField

jlegrone commented 8 years ago

@PhearZero there are more controlled inputs throughout the app (and which will be added later) that I'd like to not have to worry about refactoring to use value-link if at all possible. This is a bug that was only reintroduced in material-ui 0.15.x, so it's definitely fixable without changing how we build controlled components. @saikat may have some more insights.

PhearZero commented 8 years ago

@jlegrone Yea he has the base forms setup correctly. The forms that had been implemented post @saikat had been bound improperly, and judging from the history it looks like it was because of the material-ui bug.

The react-formal requires a "model" bind for it's value. It uses it internally via topeka in order to two-way bind the "model" value to the Form.Field values.

Material-ui has been patched a while back and allows for binding to the value property as long as it is not a child of react-formal since that is reserved for topeka bindings. (Some of the stand-alone Field Bindings are still buggy in GC but not in local testing or on the main material-ui docs)

Form.Field from react-formal will not work properly outside of a react-formal Form unless you use react-link or any other two-way binding.

Please let me know if you have any issues/questions on the current binding issues. I just patched them to work as is but we could/should standardize around the base forms now that material-ui is patched. I would probably vote for switching to a base form component that supports material-ui such as react-schema-form. It would allow for the passing around of data models since JSON Schema draft 4 is a standard hyper-schema/schema.