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

autoCursor broken starting in v4.0.0 #63

Closed
glasserc opened this issue Mar 2, 2018 · 8 comments
Closed

autoCursor broken starting in v4.0.0 #63

glasserc opened this issue Mar 2, 2018 · 8 comments

Comments

@glasserc
Copy link

glasserc commented Mar 2, 2018

Refs rjsf-team/react-jsonschema-form#851

We are using an UnControlled component with autoCursor={false} because this seems the most similar to the v2.X behavior. Without autoCursor={false}, every edit causes the cursor to jump to the end of the document (probably because we're resetting the value on every render). With autoCursor={false} in v3.0.7, the cursor behaves normally (for instance, advancing as the user types). However, in v4.0.0, even with autoCursor={false}, the cursor jumps to the end of the document at the end of every edit, as previously. As far as I can tell, autoCursor is essentially useless now (although I am not an expert in CodeMirror or even React so maybe I'm missing something).

I believe the change happened in 934521a#diff-168726dbe96b3ce427e7fedce31bb0bcL521.

Do you have advice on how best for us to upgrade to v4?

@scniro
Copy link
Owner

scniro commented Mar 3, 2018

@glasserc Hey thanks for opening this up. I've read through this as well as the referenced issue and am curious as to why you are resetting value on an UnControlled component. The uncontrolled component does't care about the value whatsoever other than a starting one, as it's all managed internally and simply spit back out to the user on a cb (if needed). It's best to just give value a static string starting value and never worry about it again.

Perhaps you are wanting the Controlled variant if you are indeed wanting to manage value (perhaps this is coming down via redux, or similar). A few solutions come to mind...

  • Try not resetting the value prop.
  • If ^ is not possible, try using a Controlled component and manage the prop however you're managing state in your app
  • On my end, I can be more strict within componentWillReceiveProps on the UnControlled component.

I'm thinking of doing the last suggestion anyways as it makes sense, but I'd try the first two as well as I suspect you may be using UnControlled just a bit incorrectly. Either way, keep me posted with your findings and If I can reproduce this locally I'll get a fix published this weekend.

@glasserc
Copy link
Author

glasserc commented Mar 3, 2018

Hi, thanks for your advice. Some background: in our application, the user edits JSON in the editor, but periodically (when the JSON value being edited "changes", i.e. the parsed JSON object is different from the last parsed JSON object) we re-set the value to a pretty-printed version of the current JSON object. When this happens, ideally the cursor would stay "where it is", i.e. if the user has just typed the closing quote after a new property, then the cursor would still be located at this closing quote, even if the closing quote moved to a new location due to pretty-printing. However, this behavior seems hard, and all I really want is to have behavior that is similar enough to our old behavior, where the cursor goes to someplace nearby.

I think you're right that we are actually most like a Controlled variant, but I had some problems with the cursor moving to the end of the value whenever we re-format the JSON value. So I guess my bug should actually have the title "How can I maintain cursor position when I update the value?" As you can see from the linked issue, I tried using autoCursor={false} with the Controlled component, but that caused even stranger behavior -- the cursor remains fixed, even when typing new characters (which appear after the cursor, causing typed text to come out backwards). We weren't tracking the cursor position as a prop previously, but maybe that's the approach I should be following? I tried something like this but didn't see any difference in behavior from just not tracking the cursor state at all.

@scniro
Copy link
Owner

scniro commented Mar 3, 2018

Hm okay, it seems like option 3 may be the best quick approach here. You can confirm this was not an issue in 2.x? I think I just need to be more strict in the handling of props on the uncontrolled component

@scniro
Copy link
Owner

scniro commented Mar 4, 2018

@glasserc okay I may have found the issue by being more hands off with the uncontrolled component and letting cursor delegation largely be driven on it's own unless explicitly specified via autoCursor. The behavior should replicate the likes of v2.x

Here's the deal, testing this lib has been tough, and I can't reliably run my suite and assume this safely fixes the issue with the upmost certainty. I'm often left to manually regress this, which is okay for now, but it gets very challenging because there are a lot of different use cases from folks, some I have never even imagined before, lol. Also, even with reading over your use case a few times I couldn't exactly reproduce this on my end so I worked off a more basic scenario.

I pushed up scniro/react-codemirror2-test-pkg so this can be tested in isolation. Can you please install this and let me know if any of the issues on your end are resolved? Thanks

@glasserc
Copy link
Author

glasserc commented Mar 4, 2018

Yes, the behavior from 2.x is the same as the behavior of 3.x's UnControlled with autoCursor=false, and it's the same behavior I see from your test package (i.e. it fixes my issue, thanks!). I have a lot of sympathy for how difficult it is to maintain a project where people use it in unexpected ways -- if there's a more expected way for us to do what we're doing, I'm totally willing to do that too. I just couldn't figure out what that way was. If you have other branches or tests you'd like me to try, I'm happy to do that as well.

@scniro
Copy link
Owner

scniro commented Mar 4, 2018

@glasserc Excellent news! I'm happy to hear this. Ideally I'll have feature branching in the project to just pull the source files but I've found folks who I can kindly convince to help me prefer quickly installing the tgz file 😸. Either way I definitely appreciate your help on this. I'm going to regress this a bit more this evening and publish up a new version for you to consume. Will keep you posted here when it's available.

scniro added a commit that referenced this issue Mar 4, 2018
@scniro
Copy link
Owner

scniro commented Mar 4, 2018

@glasserc You should be all set with the 4.1.0 release. Please let me know if this fully resolves your issue as you tested with the earlier patch. Thanks for the collaboration on this!

glasserc added a commit to rjsf-team/react-jsonschema-form that referenced this issue Mar 5, 2018
* Move to the 3.x series of react-codemirror2

* Move to the 4.x series of react-codemirror2

See scniro/react-codemirror2#63 for more details.
@glasserc
Copy link
Author

glasserc commented Mar 5, 2018

Yes, it works great, thanks again!

@glasserc glasserc closed this as completed Mar 5, 2018
jnorris-carecloud added a commit to CareCloud/react-jsonschema-form that referenced this issue Jul 2, 2018
* Fix rjsf-team#221: No validating but updating errorSchema base on array operation when live validation is off.

* Fix comparing

* Directly update errorSchema in onChange

* Typo fix (rjsf-team#737)

* Make form submission example clearer (rjsf-team#736)

In the example, the `onSubmit` function receives a `formData` obj. However, the actual data is in `formData.formData`. While this is explained in the docs, I propose making it clearer in the example as well.

* Update prettier to v1.8.2 (rjsf-team#756)

* fix: Pass `disabled` prop to `FieldTemplate`. (rjsf-team#741)

If you want to change how your FieldTemplate is rendered based on
ui:disabled, you currently have to check `uiSchema["ui:disabled"]`,
which is unlike the pattern for `readonly` and other known `ui:*`
settings.

* Fix issue rjsf-team#747 (rjsf-team#748)

* 66 enum no type (rjsf-team#668)

* Failing test for enum without type (rjsf-team#66 rjsf-team#611)

* Proposed fix for enum without type (rjsf-team#66 rjsf-team#611)

* PR feedback - getSchemaType function

https://github.com/mozilla-services/react-jsonschema-form/pull/668/files#r133395455

* Fix onAddClick signature in ArrayFieldTemplate (rjsf-team#775)

According to https://github.com/mozilla-services/react-jsonschema-form/blob/master/src/components/fields/ArrayField.js#L224 `onAddClick` is not returning function as specified in the documentation.

* fix typo in css code (rjsf-team#799)

* README: fix code snippet (rjsf-team#785)

* Generating idSchema based on dependency changes Fix rjsf-team#778 , Fix rjsf-team#803

* Add idPrefix option (Fix rjsf-team#796) (rjsf-team#806)

* Make .editorconfig valid (rjsf-team#807)

* typo (rjsf-team#811)

* Add span and class to label required symbol (rjsf-team#765)

* a priori should be italicized for readibility (rjsf-team#825)

* Priori should by prior, I think

* Italicizing a priori based on suggestion

* pass raw errors to  field widgets (rjsf-team#826)

* pass raw errors to  field widgets

* fixed formatting

* fixing line endings

* Pass raw errors into Field at creation; object destructuring; added test for passing of raw errors; removed unused prop from ArrayField.

* fix lineEndings

* Bump 1.0.1

* Move to the 3.x series of react-codemirror2 (rjsf-team#857)

* Move to the 3.x series of react-codemirror2

* Move to the 4.x series of react-codemirror2

See scniro/react-codemirror2#63 for more details.

* Pass formContext to ArrayFieldTemplate when rendering fixed array (rjsf-team#858)

* handle errors to correctly display schema errors in form (rjsf-team#864)

* Bump version 1.0.2

* Link to official JSON Schema site (rjsf-team#873)

* Updated README to mention the support from JSON Schema compliant drop-down enums (rjsf-team#882)

* Add passing of raw errors to ArrayField template and components (rjsf-team#876)

* Fixed multiplicative errors on schema dependencies (rjsf-team#884)

* Add idPrefix option (rjsf-team#883)

* Fix warning on FileWidget (rjsf-team#842)

* Update prettier and fix files (rjsf-team#892)

* Bump 1.0.3
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

No branches or pull requests

2 participants