Atoms

[rlivsey]

Work in progress thoughts on Atoms

Will implement #222

• render atom in renderers/editor-dom.js
• parse v0.3 mobiledoc in parsers/mobiledoc.js
• render v0.3 mobiledoc in renderers/mobiledoc.js
• support atoms in text renderer - implement support for mobiledoc 0.3.0 and atoms mobiledoc-text-renderer#5
• support atoms in DOM renderer - implement support for mobiledoc 0.3.0 and atoms mobiledoc-dom-renderer#20
• support atoms in HTML renderer - implement support for mobiledoc 0.3.0 and atoms mobiledoc-html-renderer#15
• atoms should set contenteditable to false
• parse atoms in DOMParser
• handle cursors (eg when no content before/after)
• handle deletion

Maybe:

• add default unknownAtomHandler implementation which just renders the value?

[mixonic]

If a card can remove itself, can it also save itself? It if can trigger its own destruction, it seems reasonable that it could change its payload as well.

On the other hand, part of me thinks it should not be able to remove or edit itself. The most conservative approach is that they can only be created and if they are removed it must be through normal editing operations like backspace and delete. Thoughts?

[rlivsey]

I took out all the other ones from cards because save/cancel/edit/section didn't seem to be needed for atoms at first glance.

I can picture a use-case for remove such as showing an x on hover and letting you click it to remove the atom, but certainly happy to leave that out initially!

If we do have remove, I can think of some use-cases where an atom could be expose an interface which alters it and therefor would want to update its payload. Not something I personally need right now hence why it didn't spring to mind initially.

[mixonic]

remove with an x implies that the atom is (partially) in charge of its editing interface. I'd prefer to lean on the conservative path for now and ensure they have zero control over their editing interface.

We can revisit as we get more complex use-cases, if that seems reasonable to you as well.

[rlivsey]

👍

[mixonic]

I know this is annoying, but let's change this to follow the new card schema as suggested in #228 (comment)

// package: mda-profile-dom
export default {
  name: 'profile',
  type: 'dom',
  render({element, options, env, text, payload}) {
  }
};

[mixonic]

Looks good to go :-) @rlivsey I presume we will just leave it open while you spike on implementation this week.

We should attempt to preserve the 0.2 compatibility as we make changes. It will be a bit more painful in the short term since the versioning of parsers/renderer will need to be implemented, but will allow us to merge patches incrementally.

[mixonic]

should likely teardown here

[mixonic]

This can just be removed.

[bantic]

This looks good — probably the first set of challenges will be breakage caused by methods in models/_markerable expecting to always encounter markers.
The tests are a little thin around this area, but the unit tests for markupSection would be the right spot to add some more tests that those methods work properly when there is a mix of atoms and markers.

[rlivsey]

Getting close…

[mixonic]

"JavaScript" nit pick :-p

[rlivsey]

Atoms now have ZWNJ's around them, so when in a section on their own the cursor appears correctly.

Next up is figuring out that the cursor is directly before/after the atom so we can jump across when hitting left/right. For cards this is easy enough as the whole section is a card and so position.section.isCardSection is all that's needed.

Detecting if you're next to an atom is a little trickier by the looks of it.

The atom's DOM looks like:

<span>
  ZWNJ
  <span class="-mobiledoc-kit__atom">...</span>
  ZWNJ
</span>

If the cursor is right after the atom, or in the ZWNJ, then it looks like this.cursor.selection.anchorNode is the ZWNJ and we can do something like this to see if we're in the atom:

let renderNode = this._renderTree.getElementRenderNode(anchorNode.parentNode);
if (renderNode && renderNode.postNode.isAtom) {
   // ...
}

If the cursor is directly before the atom however, it's not in the ZWNJ, at this point we could do something like:

let _comparePosition = comparePosition(this.cursor.selection);
if (_comparePosition.tailNode.length === _comparenPosition.tailOffset) { // at the end of the node
  let nextNode = _comparePosition.tailNode.nextElementSibling;
  let renderNode = nextNode && this._renderTree.getElementRenderNode(nextNode);
  if (renderNode && renderNode.postNode.isAtom) {
     // ...
  }
}

Any thoughts on a nicer way of handling this?

[bantic]

I am finding one outstanding bug that I'm working on fixing:

• select only atom, make it bold (=> rendering errors/inconsistencies, e.g. type character at left side of bolded atom)

And have fixed the following bugs that now need test coverage:

• type text between two atoms
• before 1st atom: type text (ensure cursor is correct place)
• after last atom: type text (ensure cursor is in correct place)
• hitting left arrow in at start of 1st atom in section (should go to end of prev section)
• hitting right arrow at end of last atom in section
• hitting right arrow at left of last atom -> cursor does not move to right side of atom (cursor._findNodeForPosition was finding the wrong node for the position)
• ensure the rendered output after editor#reparse when there is text content in the atom zwnj is correct output (must mark the section as dirty so that the newly-inserted marker gets rendered)
• [ ] ctr-A does not move to start of atom when atom is at beginning of line
• [ ] ctr-E does not move to end of line when cursor is left (anywhere left of) an atom
• “Hello Bobbob” => write test to ensure that neighboring atoms are not coalesced
• FIXED, needs tests: deleting text from right side of the atom => hitting delete when 1 character after right side deletes and places cursor on left side of atom (this is fixed, but I forget how)
• select text that ends in the middle of two “Hello Bob” atoms and delete => atoms become “Hello B”. (section.markersFor does not work properly with atoms (it cuts their value in pieces))

[mixonic]

Closing in favor of #311