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

Avoid data attribute namespace conflicts #456

Closed
wants to merge 2 commits into from
Closed

Avoid data attribute namespace conflicts #456

wants to merge 2 commits into from

Conversation

vanillajonathan
Copy link
Contributor

No description provided.

@Ionaru
Copy link
Owner

Ionaru commented May 19, 2022

What problem does this solve?

Also would be a breaking change.

@vanillajonathan
Copy link
Contributor Author

The problem it solves is it avoids any potential conflicts when using other libraries and code that might also use data attributes by putting all EasyMDE-related data attributes in its own namespace.

This is also done by Bootstrap.

Data attributes for all JavaScript plugins are now namespaced to help distinguish Bootstrap functionality from third parties and your own code. For example, we use data-bs-toggle instead of data-toggle.

https://getbootstrap.com/docs/5.2/migration/#javascript

It is not a breaking change since the data-img-src (now data-mde-img-src) attribute is not part of the public API, it is not mentioned anywhere in the documentation, it is an internal concern only.

@Ionaru
Copy link
Owner

Ionaru commented May 19, 2022

Even though it is not documented, it is still part of the public API and can be used for styling purposes (see here: https://github.com/Ionaru/easy-markdown-editor/blob/master/src/css/easymde.css#L368).

There's a reasonable chance other developers have used that attribute to hook in extra functionality or style an element in the 2 years data-img-src has existed.

@vanillajonathan
Copy link
Contributor Author

I see. I was not aware it was considered part of the public API. The data attribute is not mentioned in the documentation.

If you don't want to merge it, consider assigning the PR to a Project or a Label for the upcoming major rewrite.

@Ionaru
Copy link
Owner

Ionaru commented May 19, 2022

Ref #447

@vanillajonathan
Copy link
Contributor Author

Yeah, I saw that post. Good that you made a ref to it. GitHub includes "Projects" and "Labels" too which can be assigned to issues and pull requests.

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

Successfully merging this pull request may close these issues.

2 participants