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

VIM - Strange behavior in VIM mode with Controlled Usage #37

Open
edmondxiexie opened this issue Nov 30, 2017 · 17 comments
Open

VIM - Strange behavior in VIM mode with Controlled Usage #37

edmondxiexie opened this issue Nov 30, 2017 · 17 comments

Comments

@edmondxiexie
Copy link

The VIM mode has strange behavior when working with states. For example, when I press either 'o' or 'i' to enter insert mode, then press Backspace. It doesn't seem to allow me to delete previous characters before the insert position, which seems weird. And also, the cursor in insert mode will change from the line shape to the block shape after one insertion operation. Is it related to the rerendering when state changing? Because it does not happen in UnControlled Usage.

I use the OnClick function to change the state and use states for changing the value in the editor.

Preview:

  • I enter the edit mode and move cursor to the end of the line.
    screen shot 2017-11-30 at 12 02 09 pm

  • Then I press Backspace and the cursor change its shape and I cannot delete any more.
    screen shot 2017-11-30 at 12 02 54 pm

@scniro
Copy link
Owner

scniro commented Dec 1, 2017

Interesting, thanks for opening this up, I'll give this a look later today. There is a bit of logic going on for controlled mode to retain/set the cursor in certain cases. I hope VIM isn't going to be a one-off to work around this. I'll keep you posted!

@scniro
Copy link
Owner

scniro commented Dec 3, 2017

@edmondxiexie Sorry for the delayed response. Okay yep I can definitely reproduce this. Can you use the uncontrolled variant for now? I'm unsure how much work this could entail. I haven't used this mode before nor am I that much of a VIM power user, so when looking for somewhere to get started I came across this on this note over on codemirror

The CodeMirror vim bindings do not have an active maintainer. That means that if you report bugs in it, they are likely to go unanswered. It also means that if you want to help, you are very welcome to look at the open issues and see which ones you can solve.

This has me a little hesitant to dive in as it could lead to an unraveling of who knows what. Either way, I'll start to look and keep you posted.

@scniro
Copy link
Owner

scniro commented Dec 4, 2017

@edmondxiexie okay so I dug into this a bit and think I may know whats going on here. Basically, for the controlled component there is also a dummy DOMless codemirror instance that mirrors and tracks changes, to which is later applied to the main instance upon state management (new props). However, when using VIM mode (and likely others), there is a notion of a key map that alters the behavior of the editor e.g. for insert => press "i"

The issue is, I don't believe I can emulate this key event to the mirror (dummy), so when I go ahead and extract the mirrored value, it's now incorrect because the keymap altered what would have been applied to the main instance, and is something different entirely. So what I've been digging for is a way to trigger a keystroke change programatically to the dummy, but I have been having trouble to find such a way.

There is a keyHandled callback which is an absolutely perfect hook to apply the emulated event to the dummy, however I'm struggling to find a way to do so through the APIs. I thought maybe I could even grab the native <textarea> and manually dispatchEvent to it - but I can't get _the dummys instance to fire keyHandled.

I might need help with this one, but I'll keep at it when I have time.

@xralphack
Copy link

same issue on both Controlled and UnControlled

@scniro
Copy link
Owner

scniro commented Feb 9, 2018

@xralphack odd, this shouldn't be an issue for uncontrolled

@xralphack
Copy link

@scniro you are right, just ignore my comment

@scniro
Copy link
Owner

scniro commented Feb 12, 2018

@xralphack Cool - no worries. Yea I haven't been able to make a lot of forward progress on this one. It's pretty gnarly on the inside of whats going on as you can see in my investigation above, so hopefully the uncontrolled component is a decent workaround for you, for now.

@xralphack
Copy link

@scniro I figured out the problem

I want to wrap the codemirror2 component

// wrapper component

<MarkdownEditor
  markdown={originMarkdown}
  onMarkdownChanged={markdown => this.setState({ markdown })}/>

// codemirror2 component

class MarkdownEditor extends Component {
  ...
  render() {
    return (
      <CodeMirror
         ref="cm"
         value={this.state.initialMarkdown}
         options={{
           keyMap: 'vim'
         }}
         onChange={(editor, data, value) => {
           this.props.onMarkdownChanged();
         }}
    );
  }

use uncontrolled version not work, I think this is due to the parent component rerender
so after add the following, it works fine.

shouldComponentUpdate(nextProps, nextState){
  return false;
}

anyway, thanks for your effort.

@scniro
Copy link
Owner

scniro commented Mar 13, 2018

@xralphack shouldComponentUpdate for UnControlled? Interesting, very. I use this lifecycle hook when responding to new props via the uncontrolled component for resetting values, setting a new theme, etc. Do you think "freezing" the component once it's mounted would solve this via freeze={true}, or something? (Unsure on naming) That prop would essentially do exactly as you shared and prevent any updates. I think that could be valuable if a user actually wanted that behavior, as your use case is showing. Please let me know if that makes sense?

@xralphack
Copy link

@scniro I think using freeze makes sense, but I am not sure using boolean prop is ok for all conditions

@scniro
Copy link
Owner

scniro commented Mar 14, 2018

@xralphack why would it not be okay? shouldComponentUpdate () => false essentially never updates. I am going to add a detachOnMount boolean that does this

@xralphack
Copy link

maybe someone want to toggle this boolean on specific condition ( I'm not sure)

@scniro
Copy link
Owner

scniro commented Mar 14, 2018

@xralphack gotcha. So I'm thinking then of a detach={variable} that can be bound, so whenever it's truthy the component will not update at all, but when false will behave as normal. Does that sound better?

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

scniro commented Mar 14, 2018

@xralphack okay there is now a detach prop. This should both prevent the component from updating via shouldComponentUpdate lifecycle event and prevent any internal option setting. I may break out the latter option setting to another prop such as detachInternal, but unsure yet as I'd like to gather feedback on how this is used. The new prop also ships with editorDidAttach and editorDidDetach for good measure. These are now available in 4.2.0 and only for the UnControlled component. I hope you find this helpful.

@joeldouglass
Copy link

joeldouglass commented May 8, 2018

I was able to successfully dispatch the key maps to the mirror like so:

this.editor.on('keyHandled', (...a) => {
  console.log('Editor', a);
  const ke = a[2];
  this.mirror.display.input.textarea.dispatchEvent(new KeyboardEvent(ke.type, ke));
});

(note: I had to use the spread operator to get all three event arguments and avoid a typescript error. I'm not sure what that is all about).

This seems to correctly dispatch to the mirror, because I can listen for keyHandled events on the mirror and watch them fire at the same time. The mirror also correctly fires the vim-mode-change event at the correct time!

However... the behavior is still exactly the same as described in the original bug. So no luck yet.

See below the results of logging the keyHandled and vim-mode-change events on both the master editor and the mirror version.

image

@taniarascia
Copy link

Have there been any updates on this? Vim mode is unusable in this component...

@scniro
Copy link
Owner

scniro commented Jan 19, 2020

@edmondxiexie @taniarascia @joeldouglass @xralphack I am a lot shorter on time these days as when I started this project. Codemirror & React APIs are moving to quickly for me to keep atop of for the day-to-day. I am looking for a co-maintainer of this project. Please contact me directly if you are interested. Thank you for understanding.

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

5 participants