scniro / react-codemirror2

Codemirror integrated components for React
MIT License
1.65k stars 192 forks source link

Undo / redo does not fire onBeforeChange() in controlled component #31

Closed cephirothdy2j closed 6 years ago

cephirothdy2j commented 6 years ago

When making a change within the editor, and hitting CMD-Z to undo your changes, the onBeforeChange() event does not fire. However, the onChange() event does fire.

I'm willing to use onChange() to get around the issue, but all of the documentation around the controlled component states that onBeforeChange() should be used for managing the value of the component.

Here's a GIF of the problem in action:

onbeforesaveundo

Easiest way to reproduce:

Do as I did in the GIF: Visit the provided example with the React DevTools extension opened in Chrome, and you can see that the value of the input does not update in component state (done via onBeforeChange); though you do see the change event firing in the console, and the text within the component reflects what we see in the console.

scniro commented 6 years ago

Hm interesting, thanks for bringing this up. I know codemirror manages some form of state internally in regards to history so the undo updating you're seeing if mostly codemirror itself.

I do think using onBeforeChange is appropriate here. Will keep you posted!

cephirothdy2j commented 6 years ago

Thanks for the quick response. FWIW it's not included in the GIF above, but once you CMD-Z, not only does it fail to update the component's value immediately; any key input after that point places the key input in the wrong spot within the value and you end up with a completely broken experience.

scniro commented 6 years ago

@cephirothdy2j Hey again, this should be fixed with the 3.0.4 release. Working with the codemirror history api proved to be quite difficult, but I think I was able to get through it. Can you please try the new release out?

Also, the side effect you had mentioned should be resolved, I too ran into that and think this fix should squash it.

cephirothdy2j commented 6 years ago

Thanks for looking into this so quickly. I feel your pain on working with codemirror history; I forked your repo and attempted to fix this morning but could only fix about 60% of the problem. e.g. I'd get the undo stack working but lose the redo stack at the same time, or changes would undo correctly but misplace the cursor.

3.0.4 is MUCH better than 3.0.3 but there is still an issue when "fast-undoing". See the attached GIF, where I'm making some changes in the example and then holding down Cmd-Z to undo them all. One character gets left behind, and the codemirror instance becomes out of sync with your controlled component. The result is I can't even delete the leftover character unless I delete the entire line.

fast-undo-issue

scniro commented 6 years ago

@cephirothdy2j Yep,I too can reproduce this. I don't think it has to do with the speed as I can get the same result still by slowly undo/redo-ing all of the inputs as well on your <div> example, but maybe the auto indentation codemirror is doing - specificially on electricInput. Unsure fully yet...

I'll be working through this today and will share my findings with you. Thanks for the collaboration!

scniro commented 6 years ago

@cephirothdy2j Wow, this was tough! Thanks again for all the help on this. I believe I finally got this working with the 3.0.5 release. Let me know if better? Please reply back here if you should notice anything else weird again, but I'm feeling confident you'll get the behavior you're expecting this time. The only lingering thought might be an inconsistency in the cursor position so be sure to pay careful attention to it 🤞

cephirothdy2j commented 6 years ago

It's perfect! I can't recreate any of the previous issues and so far I haven't seen any cursor weirdness.

Looking at the commit, you had to wade pretty deep into Codemirror to get to the root of the issue. Thanks for looking into this and resolving it so quickly.

scniro commented 6 years ago

Np 👍 Going to close this out for now. Please re-open should you notice anything odd and thanks for using react-codemirror2!