Skip to content
This repository has been archived by the owner on Jun 15, 2021. It is now read-only.

Missing core patch for "bold" and "italic" commands #461

Open
mindplay-dk opened this issue Mar 15, 2016 · 1 comment
Open

Missing core patch for "bold" and "italic" commands #461

mindplay-dk opened this issue Mar 15, 2016 · 1 comment

Comments

@mindplay-dk
Copy link

Looks like we need core patches to properly support <strong> and <em> tags - the strong and italic commands do not work as expected.

In the latest version of Chrome, the queryState() method for these two commands appear to depend on CSS artifacts, e.g. whether the content happens to inherit the CSS rules font-weight: bold or font-style: italic.

You can verify this in the demo, by simply adding one of these CSS attributes to the editor <div>, and you will see the "bold" and "italic" buttons on the toolbar appearing as active.

I don't know why it behaves this way, but it's clearly wrong - whether your CSS happens to apply certain presentation rules has nothing to do with the <strong> and <em> tags, in terms of semantics.

My concrete case is a <blockquote> tag, which has font-style: italic applied to it by a CSS rule - this shouldn't mean we don't get to use <em> inside it, and it definitely should not reflect on toolbar state.

The <em> tag strictly means emphasis, it doesn't mean italics - HTML5 separates the semantic meaning of emphasis from the presentation artifact of an italic type-face.

With that said, Scribe uses the standard behavior of <b> and <i> tags by default, unless patched with something like scribe-plugin-formatter-html-ensure-semantic-elements - so I guess it's debatable whether this belongs in the core, or in that plugin?

I will patch this up for my own purposes tomorrow - if you can let me know where you think this feature belongs, I can submit a PR, if you'd like?

@mindplay-dk
Copy link
Author

Hmm, attempted a quick patch by adding this to patches/commands/bold

      boldCommand.queryState = function () {
        var selection = new scribe.api.Selection();
        var boldNode = selection.getContaining(function (node) {
          return node.nodeName === 'B' || node.nodeName === 'STRONG';
        });

        return scribe.api.CommandPatch.prototype.queryState.apply(this, arguments) && !! boldNode;
      };

That does not appear to be enough. The default behavior of executing the "bold" command results in the creation of a <span style="font-weight: normal;"> node - yikes.

So looks like we'll need a custom implementation of execute() for the "bold" and "italic" commands.

Alternatively, maybe we should consider adding new, entirely custom commands named "strong" and "em"? (Though I doubt anyone would actually want the "bold" and "italic" commands as-are, for any reason? I can't really imagine anyone is using scribe without the ensure-semantic-elements plugin?)

I will look into this some more tomorrow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant