search

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

Refactor into using a map for icon classes #454

Closed vanillajonathan closed 2 years ago

vanillajonathan commented 2 years ago

@Ionaru Please merge!

Ionaru commented 2 years ago

Firstly, this is a project I maintain in my free time next to my day job. I look at all the issues and PRs, some take longer to look through and make a decision for. Please allow me the time for that.

Secondly, I'm not seeing the use of this change. Sure it makes a neat little variable containing all icons, but ends up being more code for the exact same effect. I rather like the current look of the toolbarBuiltInButtons variable as it shows how users can add their own custom buttons.

vanillajonathan commented 2 years ago

This PR serves as the underlying foundation for the next step.

Which is:

options.iconClassMap = extend({}, iconClassMap, options.iconClassMap || {});

Which allows integrators to easily use other icon sets without having to recreate every button themselves.

Ionaru commented 2 years ago

Then build what you want to build and provide the completed thing as a PR. Smaller increments through PRs only increase the risk of half finished functionality ending up in the source code.

vanillajonathan commented 2 years ago

Can this be merged?

Ionaru commented 2 years ago

I would prefer the iconClassMap to use the name property of the toolbar buttons because that is what the toolbar option uses as well.

vanillajonathan commented 2 years ago

Alright, done.

willGabrielPereira commented 10 months ago

Hi @Ionaru, this PR was made for exactly what? because it keeps doing the same effect, you can't change the icons even after this changes.

The https://github.com/Ionaru/easy-markdown-editor/pull/501 looks like really fix this. Please take a look in it!

vanillajonathan commented 10 months ago

@willGabrielPereira This PR was never meant to let you change the icons, it was just a refactor and a stepping stone to later expose it as options. See the comment https://github.com/Ionaru/easy-markdown-editor/pull/454#issuecomment-1133860132 above.