Retain attributes after deleting

[John Brecht]

This is somewhat related to the "new line cancels attribute toggle off" topic below. I would like to modify Firepad such that if you apply some attribute, say Bold, and type some text on a line, then delete all you've typed and start typing again, you lose the attribute and get plain text. Is there a setting to retain that? If not, any suggestions as to where I should insert such a modification into the code?

[John Brecht]

E.g. suppose I make this change in RichTextCodeMirror.prototype.updateCurrentAttributes_:

    var spans = this.annotationList_.getAnnotatedSpansForPos(pos);
    var oldAttributes = this.currentAttributes_ || {};
    this.currentAttributes_ = {};


    var attributes = {};
    // Use the attributes to the left unless they're line attributes (in which case use the ones to the right.
    if (spans.length > 0 && (!(ATTR.LINE_SENTINEL in spans[0].annotation.attributes))) {
      attributes = spans[0].annotation.attributes;
    } else if (spans.length > 1) {
      firepad.utils.assert(!(ATTR.LINE_SENTINEL in spans[1].annotation.attributes), "Cursor can't be between two line sentinel characters.");
      attributes = spans[1].annotation.attributes;
    } else {
      this.currentAttributes_ = oldAttributes;
      return;
    }

Is that going to break something else? All it does is if there's nothing to the left AND nothing to the right, then just use the previous attributes. (And if there were no previous attributes, attributes is {}).

[Michael Lehenbauer]

Hey John,

Does that actually give you the behavior you want?  I haven't dealt with that code in a while, but it seems like that code would only have an effect if you're on a blank line at the very end of the doc (i.e. the attributes to the left are line attributes and there are no attributes to the right).

I could be confused though.

-Michael

Confidentiality Notice: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please notify the sender by reply e-mail or phone and delete the original message and its content, including all attachments, if applicable.

--
You received this message because you are subscribed to the Google Groups "Firepad" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firepad-io+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/firepad-io/cd13f318-c793-4017-b0f9-5879fe43c7b3%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[John Brecht]

No, this does not actually handle it correctly, only works for the first line. I guess I'm asking in general, is this a tack that could work safely, assuming I get the condition right? But then yeah, the real question is, what's the condition? For sure the condition includes being at the beginning of an empty line/paragraph, so we need to detect that. But then there are a variety of ways you could get there:

1. You just pressed enter after typing a previous line. In this case, you should hang on to previous attributes (or look backwards to the last line of the previous span.)

2. You just deleted all the content on the current line. Again, in this case, you should hang on to the previous attribute.

3. You just clicked on an empty line in the middle of your document. Then you should pick up whatever the attributes were the last time the cursor was on that line. Under the current implementation, that's not what happens though. The style is dependent on the style of the end of the last non-empty line above that line. So in practice, if you change the formatting of the line above this blank line, the blank line will pick up that change instead of retaining whatever format it had when you created the line.

Does that cover all the cases?

[Mika Genic]

as far as I can tell https://github.com/firebase/firepad/pull/241 solves this
can you check pls ?

[John Brecht]

Unfortunately, that does not address my case Mika. My case works like this:

1. Start on a new empty line of text (doesn't matter if it's the first thing in a document, or somewhere in the middle.)

2. Apply some set of attributes X, Y, Z (size, style, color, etc..)

3. Type some stuff. Attributes X, Y, and Z are applied.

4. Delete what you type until you are back at the beginning of the line.

5. Type stuff again

In the current implementation, instead of your chose styles for the line (X, Y, Z) being applied, if the cursor is at the beginning of a (now) empty line, we apply the attributes of the end of the previous line. This makes sense if you've gotten to the beginning of a blank line by pressing enter after some other line, but does not make sense if you've gotten there by deleting the content of the current line.

[Michael Lehenbauer]

Hey John,

FWIW:

1) To answer your original question, I think this is a task that could work safely.  If we make tweaks, I'd aim to just try to test thoroughly and perhaps compare the behavior to a well-established rich text editor (e.g. Google Docs or Microsoft Word).

2) Is there a reason you're special-casing the beginning of a line?  I think the logic you describe should probably happen anywhere.

3) I'm not sure, but the right approach may be to avoid calling updateCurrentAttributes_ when pressing backspace (and perhaps in other situations too, I'm not sure).

-Michael

Confidentiality Notice: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please notify the sender by reply e-mail or phone and delete the original message and its content, including all attachments, if applicable.

--
You received this message because you are subscribed to the Google Groups "Firepad" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firepad-io+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/firepad-io/73e56246-b2b6-44e3-9d47-060544b84238%40googlegroups.com.

[John Brecht]

1) I'll mess with GDocs and Word to confirm what they do

2) Special-casing it because the behavior mid-line is less jarring. But yes, you are correct it's not exactly "right" per the definition I'm now applying here. E.g. if you typed something like:

NormalBoldItalics

And then pressed the delete button until you got back to:

NormalBold

Then technically, yes, the next key you press should be italics, but it's bold. Honestly, you could slice that one either way and it seems reasonable to me. But for some reason it's more jarring if I have a pair of lines like:

Bold

Italics

and then I press delete until I'm at the beginning of line 2, and then I start typing again and get bold faced characters, it seems wrong.

3) I'm not sure if that is exactly right, have to think about it. But, in the cases I outline above, where you have multiple styles within a line and you backspace back through them, then resume, you should resume with the style of where you are. Although... I guess that first keypress you type to insert a new character after deleting will take care of it, won't it?

[Mika Genic]

ok I see now :)  also checking in for example gmail editor this is indeed the desired behavior

ATM updateCurrentAttributes_ determines attributes only based on what's to the left/right and not the current 'state'. would be good to check if your change integrates with https://github.com/firebase/firepad/pull/241 as that PR solves two other issues with the same code.

thx!

[Michael Lehenbauer]

For what it's worth, my intention with the updateCurrentAttributes_ method was that it would be called to figure out what attributes you should have after cursor moves (via the cursor keys or clicking somewhere random in the editor).  And so it intentionally only looks at the stuff to the left/right of the cursor.  I think CodeMirror fires cursor change events on just about any type of edit action though, so my initial thought would be that calling updateCurrentAttributes_ in fewer cases would be the preferred approach to solve John's issue (and perhaps Mika's too) rather than complicating the updateCurrentAttributes_ logic.  Though I admit I haven't fully digested what you were trying to accomplish Mika.

-Michael

--
You received this message because you are subscribed to the Google Groups "Firepad" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firepad-io+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/firepad-io/0dd55cd9-292d-4369-9efb-4f8f748c63be%40googlegroups.com.

[Mika Genic]

also John considering your last msg above

Playing with gmail editor I see in your case 2 that gmail will print the next letter as bold & italics, that is it will combine the bold from the char to the left and the italics from the current state. I don;t think this is great. FP with 241 will only pick up bold.

The clearer behaviour for me feels that in this case new chars should only be italics, that is that the rule should be -

    if there are no chars to the right keep the current attribute state

does that sound like what you're after ?

[John Brecht]

Yeah, I think that should cover all the cases where you arrived where you are via typing (delete or return), but not necessarily by navigating (clicking or arrowing.) Let's say you've been typing some document in a variety of styles and you've left an empty line in the middle somewhere. What style should you get if you click on that line or arrow down to it? One answer would be to use the style of the first non-empty element you find moving upstream from there, which is in fact what is currently done I think.

I almost feel like there should be a line-level 'default' style. That style would start out as being whatever style you had when you finished typing the previous line and hit enter to get to this one. However, it should get overrided as soon as you style the first span in this line to anything else. If you delete that span, that style remains around as the 'line' style. But maybe that's making things to complex. I guess if we want to keep things mostly as they are, but get at the functionality I'm looking for, we would have to do what I laid out above, using the new rule you defined, but with the addition of distinguishing as to whether you've arrived at the beginning of this empty line by typing or by navigating. Is that possible?

[Mika Genic]

yes you're right. and the rule I suggested is wrong.

In the case you describe - having a few style on and then navigating to a new place, I think https://github.com/firebase/firepad/pull/241 gives a satisfactory solution in all cases I found. Can you tell me if that's true ?

what we are left with is specifically the backspace issue, so the rule I'm suggesting now is:

    pressing backspace will preserve attributes

does that seems correct and solving all cases ?

however such rule may not easy to implement :( 

[John Brecht]

This is shockingly difficult. 'pressing backspace will preserve attributes' almost works, but if I backspace into something with different attributes, then we should pick up those attributes. is there any way to implement that rule?

[Michael Lehenbauer]

Oh, wow.  I assumed that was the desired behavior (to continue preserving attributes), but I guess it only preserves until where you started typing (or last changed attributes I assume)  This is starting to seem pretty tricky/annoying.  You'll need to keep track of the last location where you changed attributes, I guess.

Maybe something like:

1. In addition to currentAttributes_, store a currentAttributesSetPos_ value that keeps track of where the cursor was when the attributes were last changed.
2. Whenever we change currentAttributes_ (e.g. because you moved the cursor or because you actually changed the current styling), set currentAttributesSetPos_ as well.
3. Either in the backspace implementation (or maybe it's safe to just do it in the general updateCurrentAttributes_() method), if the current position is the same as the currentAttributesSetPos_, then keep currentAttributes_ instead of recalculating based on the surrounding text.

Might run into other edge cases, but it seems like that might work...

-Michael

Confidentiality Notice: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please notify the sender by reply e-mail or phone and delete the original message and its content, including all attachments, if applicable.

--

You received this message because you are subscribed to the Google Groups "Firepad" group.
To unsubscribe from this group and stop receiving emails from it, send an email to firepad-io+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/firepad-io/e9b8af2b-c6a1-4e63-9eca-18a74e0dafd7%40googlegroups.com.

[John Brecht]

well, maybe that is a compromise we can go with for simplicity's sake. to make the behavior i'm talking about crystal clear, go to a fresh Google doc and enter the following:

1. first type a word in Bold

2. press enter twice

3. now, on the third line type three connected words in 3 formats: ItalicBoldUnderline

Now start pressing backspace and observe which style is selected in the toolbar as you do. You'll not when you delete the entire word 'Underline' but not yet the 'd' in 'Bold', you are still in Underline mode. Once you delete that 'd', then you enter Bold mode. When you delete all the letters on the line, but are still on the third line, you are in Italics mode. Press delete once more to go to line 2 and now you are in Bold. Now, repeat all that except after you've finished, change the first line Bold style to something else. You'll notice that when you backspace to line 2, it's still Bold.

 How does this translate into Code Mirror terms? I'm not exactly sure. :) I think the deal is something like this... Let's say you are on a line with three spans with three different styles. Your cursor is at the end of the 3rd span and you start hitting backspace. you delete everything in the third span, but don't yet go into the second span. You should still keep the attributes you had from the third span. It's the act of entering the second span, by way of deleting your way into it, that the current attributes should now change to that span's attributes. Is that something we can do?

[Michael Lehenbauer]

Oh, actually I wonder if the logic is basically just "after delete, pick up the attributes of the character you just deleted."  It seems like that would work for the single-line case at least?

For the multi-line case, I think Docs is doing the same thing but it gets more interesting because I think Docs is allowing the newline (which is invisible) to be bold/italic/whatever meaning that when you delete the newline, you pick up whatever attributes it had.  I'm not sure if firepad will behave similarly...

-Michael

To view this discussion on the web visit https://groups.google.com/d/msgid/firepad-io/b55f1b9d-e7f9-4f47-8cb5-cb81df6a0ea6%40googlegroups.com.

[John Brecht]

Oh, that sounds right, the single line case anyway. And yeah, I think you are probably right about Docs treating newline as a character, and that character has a style. Since there's no such thing in CodeMirror (or really HTML generally), the next best thing is to apply a style to the <pre> that contains the line. (the one with class CodeMirror-line.) There is precedent for putting styles there, since that's where indent-level lives, but I don't know, that may be pushing the model too far.

[Michael Lehenbauer]

Well, firepad actually manages the rich-text styling completely separate from CodeMirror (since CodeMirror isn't a rich-text editor).  It's not ideal since there are essentially two separate data models (the text and the attributes) that have to be kept in sync.  But firepad could definitely track rich-text attributes for newlines, and honestly it might already.  I don't remember.

-Michael

[Mika Genic]

"after delete, pick up the attributes of the character you just deleted."

A few points:

- I assume you mean what is windows backspace ? and mac delete ? we need to also think about windows delete although that seems to work ok in FP

- This is not the gmail editor behavior which is more close to the current FP. In general I think gmail behavior there is confusing

- It does however seem to be wordpad (and prob Word) behavior. And overall it seems to me this logic gives a clear user experience. So I think we should aim for that.

- I wonder if there is any standard covering these kind of logics ? I mean this is something editors had to deal with for decades.

Any more pointers as to how to go around implementing this logic ?

Thx!

  

[Mika Genic]

I have updated https://github.com/firebase/firepad/pull/241 to solve the backspace issue

demo: http://plnkr.co/edit/VYG67MoMMtpk11x5Pr6E?p=preview 

Basically I detect backspace using keydown and prevent updatecurrentattributes in this case.

This PR now also includes changing the toolbar button state for bold/italics/underline/strike 

Please have a look and let me know.

[John Brecht]

I'm having build issues attempting to use that branch, have a look at my comments in your PR.

[Mika Genic]

thx John ! I think fixed ... pls check

[John Brecht]

This is great work! I made some more comments on the PR. Regardless of those issues, I think I want to merge this branch into my fork.