EpicEditor TAB implementation

Issue-159.md

“Smart tabs” sound better, a quick look at configuration files for both vim and sublime text:

Vim 
---
noexpandtab
tabstop
smartindent
smarttab

Sublime Text 
------------
tab_size
translate_tabs_to_spaces
use_tab_stops
...

smarttab in vim is used to respect <BS> character in some circumstances. For example if a tab key is to insert 4 spaces, a <BS> will delete 4 spaces if it is performed after smartindent/expandtab inserts 4 spaces in front of a line.

In the case of EpicEditor, we are combining some of these features into one option:

• Enable tab key (and shift-tab);
• Don't expand tab to spaces (and use css: tab-size instead) – I think expansion of tabs could be useful, but may introduce too many options/variables. But it is something we can discuss: my position is to keep it simple, at least for this first implementation. But I will experiment with this, keen to see how this will work;
• By enabling tab key, we introduce smart indentation (rule where previous lines' indentation is respected).

What I can do is try and experiment with internal options such as smartIndentation, smartTabs, expandTab and see how they pull up together. I am quite keen on seeing how others implement this first.

We can always change the name later on. I'll keep it as smartTabs for now.

If we could use iframeDocument like pre, and strip out all the different trash tags Chrome, Firefox, IE, Safari and Opera generate, things would become a lot easier.

That's the goal and I'm already basically doing this with _getText() and _setText(). The only tags I leave are <br>s. I'll explain below in response to your #2 point.

1. Paste (whether we roll our own or use execCommand, we will need to sanitise the incoming data) – my money is on rolling our own;

Yes, roll our own for sure. The _getText function in EE works great for stripping pasted text. The only problem I have is putting the cursor position in the place where the user pasted after pasting. If you can figure that out I'd love you forever :P

Here's where I finally started looking for help but still didn't get any: http://stackoverflow.com/questions/11141439/how-do-i-not-lose-position-of-cursor-on-paste-with-javascript

Also an EE ticket for this: OscarGodson/EpicEditor#100

2. Enter key – different browsers insert different tags for new lines. div and br by Chrome, br by Firefox, and etc. If we could keep pre like rendering and just insert \n character, it will be easier for us to work with in the long run as it will provide consistent textContent;

You can't just insert \n characters since it's a contentEditable which means it's just an HTML document. If you put in new lines it won't do anything because, as you most likely know, HTML doesn't care about line breaks. That's why the only HTML tag I allow is <br>s

Once we have a pure text in our self.editor, then we could easily manipulate text.

Keep in mind we'll still need to support our own HTML. Eventually we want to support Mou style editing and have nice right click context menus like Byword. This is later on in "V2", but something to keep in mind.

And about the tabbing stuff and older browsers, great that all sounds good!

P.S. It's pretty awesome you're working on this! It's something I've wanted to do but this is a lot of work. I wish you luck! I'm also going to tag @johnmdonahue (the other core contributor) in this thread so he can see all the awesome brainstorming!

Take a look at this demo on jsbin. If we emulate pre, we can use characters such as \t, \n. We emulate pre via a CSS property white-space: pre and it looks like most browsers support this.

I think textContent is not supported in IE8, but is shimmable.

P.S. It's pretty awesome you're working on this! It's something I've wanted to do but this is a lot of work.

:) I noticed that this wasn't that simple only after I started it. But it's good, lot to learn. Hopefully I can make good progress in the next week!

Oh! Great idea on that CSS trick. Just add that to the editor CSS in in the load method somewhere.

textContent sucks. I tried using it. Use innerText. Only Firefox doesn't support it and it's easier to shim innerText than textContent. Also, it'll be less work since IE5.5?, Chrome, and everyone else supports innerText.

Look how easy it is in _getText and look at the nasty shim for Firefox.

Just thinking out loud here on what needs to be done:

1. Handling of enter keys so browser specific tags are omitted (already started here feature/newline – this is based on dabblet, currently refactoring to satisfy EpicEditor)
2. Optional handling of paste command to strip out dirty html tags (feature/paste)
3. Indentation (including outdentation) (feature/indentation)
4. Remember indentation level (same branch as above)

If I nail 1 and 2, the rest should be fairly easy and straight forward!

#1 Would fix the inconsistent \r\n in Windows / IE and just \n on Windows and Mac every other browser. Now in IE you have to hit shift enter to get a single \n. It's not a big deal, but would be nice if it was the same.

Progress report, TLDR; TAB will have to wait until we get this \n sorted. The pure text version will make a lot of things easier! (I think).

Preliminary newline (\n) support is in feature/newline

Cannot test in IE7/8 atm. It looks like the develop branch isn't working on IE7/IE8 at this stage. I have tested my feature branch against IE9, Safari 5.2, Safari 6, Firefox 13, Chrome 20. There is a known issue on Opera 12 – it just doesn't work.

To do for this feature (listed in order):

1. Merge between develop and feature/newline is not pretty. Even if I rebase, I will need tinker with my code. But it needs to be done;
2. Write tests so future modifications don't break;
3. Refactor text insertion code inside newline function so that issue #162 can be addressed;
4. Change textContent to innerText;
5. Make sure new tests pass in IE8+ and Opera 12 (incl. other browsers mentioned above).

Proposed workflow for getting TAB in to main branch:

1. Finish feature/newline
2. Send pull request
3. Start feature/indentation and feature/paste and it will reuse some of the new API methods introduced in feature/newline

This will keep things branch/merging quite linear and pretty.

Yep, IE8 doesn't work yet. It's on my 0.1.2 to do list but it's been a pain. I didn't try to get it compatible with IE8 from the start so now I'm having to go back and fix a bunch of shit that could have been a lot easier if I would have done it right at the beginning. That's why from now on I'm trying to be more strict about IE8 compatible code. IE7, maybe... IE8 for sure. It will go out for the next release tho, or I plan to. Also, don't worry too much about Opera. The editor is already borked in Opera anyway.

Merging

I did a quick merge and the merge conflicts look super easy to fix. Just take whatever is in develop. Doesn't look like any code you changed is conflicting with code I changed.

Code Review

As for a quick code review, looks great. We try to stick with single var statements, but other than that it all looks consistent and very clean! Nice work! Linting should actually fail with multiple vars, but I think @johnmdonahue changed that for debugging and just never changed it back, John? I think I'll add it back to the JSHint config.

API Questions / Discussion

Your new API methods like newline, getSelectionRange, etc, are those all meant to be public methods? Like, would a non-core dev ever need those? If you think any are just for internals don't forget to put a _ at the start of the method name otherwise we'll need to document and support them and have a reason why non-internal development would benefit from those methods.

How do you feel about getSelectionEnd/Start to match the other API names which are getter and setter patterns? Something else to think about is, in the rest of the API we have get{Something}("string") like getElement('editor') or getFile('foo'). Would it make sense to stay in that same style and do like getSelection(), getSelection('start'), getSelection('end') or something similar? Thoughts or concerns?

Here's some ways to write this out

// One (current):
var selEnd = editor.selectionEnd();

// Two (half match current style):
var selEnd = editor.getSelectionEnd();

// Three (match current style):
var selEnd = editor.getSelection('end');

// Four (totally different):
var selEnd = editor.getSelection().end;

Again, super exciting and great work! Can't wait to see this land!

We try to stick with single var statements

Yep, will change I have changed these on my branch.

Your new API methods like newline, getSelectionRange, etc, are those all meant to be public methods?

Absolutely not, not all of them anyway! Yep, I've been experimenting with public API calls like insertText(text) – which inserts given text at the current cursor/selection. I will definitely clean them up. Most of the methods should be private.

How do you feel about getSelectionEnd/Start to match the other API names which are getter and setter patterns?

At this stage I am thinking of making getSelection public, but selectionStart/selectionEnd into private methods. I will also change their names to _getSelectionStart and _getSelectionEnd.

The reason why getSelection should be public is because, EE's editor (EEE?) is inside an iframe element. Normally we'd use window.getSelection, but for EE we need to make sure we select the iframeDocument. This way, people can use getSelection just like they would on a normal HTML development environment.

---

I've been working on bits and pieces here and there, between home/work so please excuse the delay! I hope to get a solid chunk of work done this Sunday! Hopefully, I can ask you for a proper code review by then!

I like insertText. And totally makes sense about getSelection. Having tried working with it working with selections is hard and IE vs other browsers handle each differently. A standardized set of tools to handle selections for developers using the API would be awesome.

No rush! Do what you can when you want :)