Add option to specify icons set #55
Conversation
|
Please don't merge, this introduces some breaks on icons I didn't check. |
|
I think it's okay. The problems was mainly with header icons, I did not check H+, H-, H1, H2 and H3. It was buggy both for fa-4, fa-5 and Material. Note that this was not specially a regression bug: these icons are also buggy on current master and dev branches. So the rendering of all icons is now: for Font-awesome 4 : for Font-awesome 5 : for Material icons : Ready to merge for me. |
Could you merge development into master then? In this way I can fix conflicts in readme on this PR. |
Done! |
Thank you! With ed96d18 this branch is up to date with master and can be merged safely. :) You should be able to use rebase and merge option to avoid creating an other merge commit. ;) |
| @@ -1145,58 +1148,114 @@ var toolbarBuiltInButtons = { | |||
| 'bold': { | |||
| name: 'bold', | |||
| action: toggleBold, | |||
| className: 'fa fa-bold', | |||
| className: { | |||
There was a problem hiding this comment.
Changing the className to this new object would introduce a breaking change for people that use the custom toolbar.
I'd rather not break compatibility at this time to make the transition from SimpleMDE -> EasyMDE as painless as possbile.
We could introduce an icon field in the toolbar button options, that looks like this:
// FontAwesome icon
'bold': {
name: 'bold',
action: toggleBold,
icon: {
fa: '<i class="fa fa-bold"></i>',
material: '<i class="material-icons">format_bold</i>'
},
...
},This way we can simply pick the correct icon depending on the chosen (or default) iconsSet.
One thing to keep in mind though is to still allow overwriting the FontAwesome icon from className (which won't work with material icons).
We might be making the API unnecessarily complicated, perhaps we should delay the feature until a full rewrite is possible.
There was a problem hiding this comment.
What about this?
'bold': {
name: 'bold',
action: toggleBold,
icon: {
'fa': {'className': 'fa fa-italic'},
'material': {'className': 'material-icons', 'textContent': 'format_italic'},
}
...
},And something like this (not tested) in createIcon():
var icon = document.createElement('i');
if(options.icon && options.icon[iconsSet || 'fa']) {
// if options.icon is provided among with a correct icon set,
// look for className and textContent attributes and defaults to fa values
icon.className = options.icon[iconsSet || 'fa']['className'] || options.className;
icon.textContent = options.icon[iconsSet || 'fa']['textContent'] || '';
} else {
// use options.className if options.icon is not provided
icon.className = options.className
}|
Is there anyway I can help adding this feature? Material Icons is already added to one of my applications. With this feature I could avoid including an extra icon set and make the icons consistent overall. |
|
It would be nice to add support for bootstrap-icons on that note too. They're licensed under MIT License and could include with their SVGs. |
|
Another approach would be a dictionary/map with key-values to define the class names to use. |
|
Hi everyone, I don't use easyMde currently so I'm probably not going to continue to work on this PR. If anyone interested, please fork my fork and create a new PR. |
This PR adds a new option
iconsSet, to let the developper specify which icons set to use.Currently, font-awesome (
fa) or material (material) are supported, but others can be easily added.Usage:
The following will produce this toolbar:
Note that font-awesome is kept by default, in order to don't introduce changes to current version.
So this:
or this
produces this, as usual: