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

Fix for #3780: Overwrite cursor #3782

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

vanderlee
Copy link

This is a fix to have a blinking overwrite cursor using minor additions in codemirror.js and styling of .CodeMirror-overwrite .CodeMirror-cursor in codemirror.css.

When state.overwrite is true, the CodeMirror-cursor's text is either the next character or, if the next character is empty, the "\u00a0" it has for insert mode. The css simply adjusts some things to put the inverted character in the cursor in the right place.

The result is a typical blinking overwrite cursor. It can be seen in action from the This is CodeMirror example in index.html, and presumably all other examples.

Works fine in Opera, Chrome and Edge, but there is a CSS issues in FireFox. I have not yet extensively tested all features and edge cases. I'd like to poll interest in this fix before continuing with it.

@marijnh
Copy link
Member

marijnh commented Jan 20, 2016

Hi. Before I look at this, please configure your editor to not change every single newline in files that you touch, and then resubmit this as clean patches, so that I can actually see what changed.

@vanderlee
Copy link
Author

I'll run a conversion. What kind of line endings did you use?
Either way travis-ci is complaining about a license blob I didn't even touch.

@vanderlee
Copy link
Author

Done

@marijnh
Copy link
Member

marijnh commented Jan 24, 2016

Much better (though you still used tabs to indent the css, ignoring the surrounding conventions).

My take is that this is too specific and fragile (try with right-to-left text, for example) to put in the core library, but it would make a great addon. I believe the library exposes all the hooks necessary to update the cursor after a selectionChange event, except for an event that fires when overwrite mode is entered or left. Do you want to refactor it into an addon?

@vanderlee
Copy link
Author

Overwrite mode doesn't work at all in RTL mode, it just inserts (or atleast that's how it works in the bidi demo). This basically makes the RTL point moot at the moment. Regardless, Bidi support is quite doable; I've added basic support to show the character on the left when RTL.

No event for overwrite mode switching is a problem, Without a way to hook into that, it'll be practically impossible. Is there any way to get around this?

The difficult problem actually seems to be the CSS; line-height is not predictable.

Do you have any good documentation on how to make addons and/or perhaps a good example of a small one that I can use as a template?

Btw. is any of the Bidi information exposed to addons?

@marijnh
Copy link
Member

marijnh commented Feb 2, 2016

No event for overwrite mode switching is a problem, Without a way to hook
into that, it'll be practically impossible. Is there any way to get around
this?

Yes, what I meant to say is that you're welcome to add such an event.

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

Successfully merging this pull request may close these issues.

2 participants