Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
stsrki opened this issue Aug 23, 2022 · 9 comments
Closed

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

stsrki opened this issue Aug 23, 2022 · 9 comments
Labels

Comments

@stsrki
Copy link

stsrki commented Aug 23, 2022

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

image

The original issue Megabit/Blazorise#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
@stsrki stsrki added the Bug label Aug 23, 2022
@Ionaru
Copy link
Owner

Ionaru commented Aug 23, 2022

This was brought up before in #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
Copy link
Author

stsrki commented Aug 24, 2022

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
Copy link
Owner

Ionaru commented Aug 25, 2022

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

@ieteo
Copy link

ieteo commented Aug 25, 2022

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: e57c16b 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 width to 100%

@stsrki
Copy link
Author

stsrki commented Sep 12, 2022

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
Copy link

PackeTsar commented Sep 13, 2022

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 Ionaru closed this as completed in d26b4e3 Sep 17, 2022
@Ionaru
Copy link
Owner

Ionaru commented Sep 17, 2022

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

@stsrki
Copy link
Author

stsrki commented Sep 17, 2022

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

@Ionaru
Copy link
Owner

Ionaru commented Sep 20, 2022

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

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

No branches or pull requests

4 participants