bustle / mobiledoc-kit

A toolkit for building WYSIWYG editors with Mobiledoc
https://bustle.github.io/mobiledoc-kit/demo/
MIT License
1.55k stars 150 forks source link

Tag names are not widely supported #451

Open eguitarz opened 8 years ago

eguitarz commented 8 years ago

I found createMarkupSection only supports a few tag names. Which is in VALID_MARKUP_SECTION_TAGNAMES ('p', 'h3', 'h2', 'h1', 'blockquote', 'pull-quote'). Shouldn't it support more tag names? For example, there's no way for me to create

 tag via editor.

bantic commented 8 years ago

@eguitarz See discussion in #342 for some additional context. Two of the goals of mobiledoc-kit are to a) have the mobiledoc format be flexible enough to be rendered in many contexts, including non-web contexts (e.g. as PDF for printing, as plain-text for SEO, as native iOS/Android views for mobile apps, as HTML-email-friendly markup), and also b) be interoperable to avoid editor lock-in. In theory it should be possible for a mobiledoc-formatted post to be renderable whether it came from a mobiledoc-kit editor, someone else's homespun editor, or, say Draft.js (if Draft.js were to add a way to serialize its document to mobiledoc format). Those two goals have led us to try to deliberately avoid thinking of mobiledoc as HTML; the section tag names are meant to be semantically descriptive rather than denote HTML element types. The interoperability goal has led us to be somewhat reluctant to allow arbitrary markup section tag names (and especially non-semantic ones such as, e.g., "centered") — mobiledoc-format posts created by editors using bespoke tag names would cease to be renderable by other renderers or editable by other mobiledoc editors.

That said, there are a few tag names that seem un-controversially useful enough that they should be included in the next version of the mobiledoc format. h4, h5, h6 and pre should all be included.

chrisdpeters commented 8 years ago

After thinking about this, I'm wondering if pre would be challenging without the ability to add line breaks. Or the editor would need to be smart enough to enter into "line break mode" when the cursor is inside of a pre element. (Even then, how would you get out of the pre element if you're stuck in "line break mode?")

Note that there is a hint that line breaks are possible in #252, which I'm looking forward to coding up for my own editor implementation.

The h4, h5, h6 would be less controversial, however. Maybe I can take a shot at adding those this weekend.

bantic commented 8 years ago

There is some discussion in #450 also about allowing programmatic control of what the enter does in different contexts — that would probably be a useful pre-requisite for pre.

The h4, h5, h6 would be less controversial, however.

A PR for this would definitely be happily accepted!

mixonic commented 7 years ago

Adding h4 - h6 will require:

These changes should likely be batched with some other format changes in the 1.0 roadmap to minimize churn of the format.

sohara commented 7 years ago

@mixonic Short of allowing arbitrary section types, would there be a possibility of adding support the the section tag name? Given that is it more semantic than presentational?

disordinary commented 7 years ago

I wonder, with this focus, if we should actually start looking at tags which are relevant to content but don't have an HTML equivalent.

For instance, commenting on content, references, or even just a generic annotation tag.

It could in itself contain a reference to a separate array within the format that contain metadata for annotations.

sdhull commented 6 years ago

I'm here because I found it confusing/surprising that I couldn't do editor.toggleMarkup('small'). Seeing the stacktrace include "isValidTagName" I thought "oh I just have to add it to a list." Wrong. 😞

IMO the goal of having a universal format while allowing custom atom & card implementations seems somewhat at odds. My homespun editor will have different atoms & cards than your editor and vice-versa. How is that different from my editor having some tags yours doesn't and vice-versa?

It's fine to have a default list and warn people that if they stray from the list they are damaging interoperability of the content their users create. But the short-list of supported tags being set in stone and requiring a PR in mobiledoc-kit & mobiledoc-dom-renderer seems unreasonably heavyweight.

Personally, I would worry more about differences between the rendering of the content in the editor & the dom-renderer. The dom renderer skips empty p while the editor displays them (this is actually a big problem for my users). There's an interesting comment that implies unknown tagnames will be rendered in a div with that tagname applied as a class in the editor (which I'm sure doesn't happen in the dom-renderer).

So I'd vote for making valid tags configurable. In the meantime, if you could point me in the right direction for how to make my small tag work in the dom-renderer, I'd be much obliged.

internalfx commented 6 years ago

@sdhull Hope this helps....

import { VALID_ATTRIBUTES } from 'mobiledoc-kit/dist/commonjs/mobiledoc-kit/models/markup'
VALID_ATTRIBUTES.push('target')
VALID_ATTRIBUTES.push('class')
VALID_ATTRIBUTES.push('alt')

import { VALID_MARKUP_TAGNAMES } from 'mobiledoc-kit/dist/commonjs/mobiledoc-kit/models/markup'
VALID_MARKUP_TAGNAMES.push('small')

Look here https://github.com/bustle/mobiledoc-kit/blob/8ffcb8a0cf5c671f8ff6a334cc3d815a15376392/src/js/models/markup.js#L6

sdhull commented 6 years ago

@internalfx thank you! That does help for the editor, however for the dom-renderer afaict there is no such easy win. I had to fork it and added a PR.

However yesterday I think I decided I was going to use the editor + editor.disableEditing() to render instead of the dom-renderer (thought I haven't actually done this yet). As I mentioned above, the discrepancies between how the editor renders a mobiledoc and how the dom-renderer renders the same source are too great IMO.

internalfx commented 6 years ago

@sdhull I needed the dom renderer to work myself. I think I have the "easy win" solution.

const tagNames = require('mobiledoc-dom-renderer/dist/commonjs/mobiledoc-dom-renderer/utils/tag-names')
tagNames.isValidMarkerType = function () {
  return true
}

If you override the isValidMarkerType function you can disable some "safety" features :wink:

You can also replace this with something that checks your preferred list of tags.