Ionaru / easy-markdown-editor

EasyMDE: A simple, beautiful, and embeddable JavaScript Markdown editor. Delightful editing for beginners and experts alike. Features built-in autosaving and spell checking.
https://stackblitz.com/edit/easymde
MIT License
2.38k stars 314 forks source link

Markdown button classnames can overlap with user-based classnames #493

Closed stsrki closed 1 year ago

stsrki commented 2 years ago

We got a report from one of our users that Markdown buttons are broken.

image

The original issue https://github.com/Megabit/Blazorise/issues/4060


Basically, it seems that the class names that are used by this library can overlap with Bootstrap or with any other user classes. They are too generic with names table, bold, italic, etc.

I have tried to change them dynamically after the MDE instance is created with the following code:

const easyMDE = new EasyMDE(mdeOptions);

if (easyMDE.options.toolbar) {
    for (let index = 0; index < easyMDE.options.toolbar.length; ++index) {
        const element = easyMDE.options.toolbar[index];

        if (element && element.name && !element.name.startsWith("mde-")) {
            element.name = "mde-" + element.name;
        }
    }
}

and it kinda works but not 100%. From what I can observe, the .toolbar is a global object, and any change to it changes all of the instances. If there are 5 editors on a page, a race condition happens, and one or two editors are rendered without the previously added "mde-" prefix.


My question is.

  1. How can I override MDE classnames? Is it even possible to do?
  2. Can MDE come with predefined classnames that include a prefix? Eg. table becomes mde-table
Ionaru commented 2 years ago

This was brought up before in https://github.com/Ionaru/easy-markdown-editor/pull/465, but I am hesitant to make this change in the current (v2) version.

Technically the class names aren't part of the public API and therefor shouldn't be breaking change, but they are very much a public part of the rendered element. I'm afraid it will break a lot of custom styling from users.

stsrki commented 2 years ago

Can it be optional? I can think of two way

  1. Add new boolean option prefixToClasses. And if true it will add mde-(or anything else). By default it can be false.
  2. Add a new field to the toolbar button/element object, eg.
{
    name: "heading",
+    buttonClassName: "mde-heading",
    action: EasyMDE.toggleHeadingSmaller,
    className: "fa fa-header",
    title: "Headers",
}

Both ways are opt-in and not breaking changes.

Ionaru commented 2 years ago

Good ideas! I think prefixToClasses will be easier to implement.

ieteo commented 2 years ago

Scoping styles by prefixing it with mde- is the way to go. Otherwise the possibility is always given that EasyMDE styles could interfere with other styles!

The braking change of button.table which interfere with bootstrap was this commit: https://github.com/Ionaru/easy-markdown-editor/commit/e57c16bc5c58302af4038a023edef436e272f7e4 which changed the button styling width from

.editor-toolbar button {
    width: 30px;

to:

.editor-toolbar button {
    min-width: 30px;

the previous styling of width: 30px was overwriting the bootstrap styling of.table which sets widthto 100%

stsrki commented 1 year ago

Hi @Ionaru, sorry to bother you. Is there any info on when we might expect a new version that would include opt-in prefixes?

PackeTsar commented 1 year ago

I'm having this issue as well. Bootstrap4 apparently has a class name conflict and sets 100% width on any elements with class="table". Implementing prefixToClasses seems like a good fix.

Ionaru commented 1 year ago

The toolbarButtonClassPrefix is now available in the @next version, this should resolve the conflict with Bootstrap.

stsrki commented 1 year ago

@Ionaru Thank you. Can you tell me where is the next release expected to be released?

Ionaru commented 1 year ago

2.18.0 has now been released and should be on npm in a few moments :)