guardian / scribe

DEPRECATED: A rich text editor framework for the web platform
http://guardian.github.io/scribe/
Apache License 2.0
3.51k stars 245 forks source link

Unbolding different weight b tags #442

Open bedeoverend opened 8 years ago

bedeoverend commented 8 years ago

It seems that if b has custom font-weight of below 600, so e.g.

b {
  font-weight: 500;
}

it won't consider selected b tag as being bolded, and the unbold command seems to fail. Can manually reproduced on the scribe example by adding in a custom style rule as above. I'm using Chrome 46.

I suspect this may be an issue with contentEditable itself, if so is a fix for this in scribe's scope?

rrees commented 8 years ago

We'll review any PRs that address the issue but it is not currently an issue affecting us.

bedeoverend commented 8 years ago

Righto, I'll look into doing a PR for it then. Cheers

eide commented 8 years ago

It's with contenteditable itself. Chrome doesn't consider font-weights under 600 to be bold, so it doesn't unbold that.

We "fixed" it by applying a class to the surrounding element that resets the weights just before executing the bold command, and then remove that class just after.

The code looks something like:

var editor = new ScribeEditor(...);
editor.use(function (scribe) {
    var boldCmd = scribe.getCommand('bold');
    boldCmd.execute = function (value) {
        scribe.transactionManager.run(function () {
            scribe.el.classList.add('unbolden');
            scribe.api.CommandPatch.prototype.execute.call(boldCommand, value);
            scribe.el.classList.remove('unbolden');
        });
    };
});

And then the css looks something like:

.unbolden {
    font-weight: normal !important;
}
.unbolden b {
    font-weight: bold !important;
}

We first tried to manipulate the elements inside the contenteditable; changing font-weights on individual elements on the fly. We found that to be way more difficult, and we couldn't get it to work correctly. It also left traces of the manipulation in the saved content, which was unfortunate.

(Instead of all this you could also just leave the font-weight of the bold tag to be the default; unfortunately that was not an option for us :wink: )

I'm not sure how to pack this up as a PR though, since it requires CSS.