Open jenlampton opened 6 years ago
As a <small>
tag is an inline format, I'd suggest to use the CKEditor Styles dropdown instead of the Format dropdown, which is for block-level text formats.
But the Styles dropdown is for adding Styles (classes) not for adding tags, no? Maybe what we should use is a button (like Bold and Italic?) Should we look at how other editors handle this?
The styles dropdown can actually provide both elements and classes. Per the help text:
Enter one class on each line in the format: element.class|Label. Example: h1.title|Title.
Unfortunately, I found it does not allow an element type only, you get a validation error if providing a style like small|Small
. But adding an (unnecessary) class works-around the problem: small.s|Small
.
Though the <small>
tag must also be specified in the tag list manually.
Seems like supporting an element without a class might solve this problem entirely.
I filed a PR at https://github.com/backdrop/backdrop/pull/2102 that allows the styles dropdown to include a tag name without a style. It looks like CKEditor supports tag names without classes just fine.
Besides "small" tags, there are a number of other inline tags that CKEditor does not provide dedicated buttons for, including:
So any of these tags would be possible support via the "Styles" button.
I also found that the Styles button includes 100% of the functionality of the "Format" button. The Styles button handles both inline and block styles, while the Format button only handles block styles. Perhaps as a follow-up issue, we should attempt to remove or deprecate the "Format" button and use the "Styles" button exclusively.
from an editor [UX] perspective, I love the idea of combining the two drop-downs. It's hard for people to know the difference, and if Styles can do everything Format can do, I don't see any good reason to keep them both. See https://github.com/backdrop/backdrop-issues/issues/2983
I also really like this approach from a site-architect [UX] perspective. When using the Format drop-down I always wonder what I need to do to add another element to that list. It's not intuitive that adding the tag under allowed HTML tags is the way to have it appear.
By switching to styles, and providing the area where we can define those styles, it becomes much clearer to how to set up which items appear in the list.
I've tested the PR in the sandbox, tried it with <code>
and <small>
, and it works. (I had to add <small>
to the Allowed HTML tags though, because it wasn't in that list.)
Added milestone 1.12.0.
from an editor [UX] perspective, I love the idea of combining the two drop-downs. It's hard for people to know the difference, and if Styles can do everything Format can do, I don't see any good reason to keep them both.
👍
PR reviewed and tested (https://github.com/backdrop/backdrop/pull/2102#issuecomment-451375977). LGTM
...there is a small problem in the code of the PR where multiple spaces have been added between the closing bracket of an if statement and the opening curly bracket.
Also, can we please refresh the PR sandbox @quicksketch (the PR was opened a year ago, so we had a couple of releases since then + CKEditor has been updated with #3355)? Besides, I would love to test this using our new fancy Options element (#1005) 😉
...also, unfortunately this has missed the train for 1.12 😞 (feature freeze was 3 days ago).
Made a new PR based on @quicksketch's one, addressing the comments made there: https://github.com/backdrop/backdrop/pull/2777
@BWPanda Quickly tried the PR in the sandbox website: I've added these tags (without class) to the Style list:
code|Code
nolink|Nolink
small|Small
I was able to use all of the tags from the Style dropdown after adding <nolink>
and <small>
to the Allowed HTML tags. (<code>
is already in this list.)
I've updated my PR with a change to the Style List description (added a note about making sure tags are added to allowed list if the HTML tag filter is enabled).
Better UX I think to automatically include these tags in the allowed tags list rather than requiring to re-write them there.
I've updated the PR based on @docwilmot's suggestion. Any tags added to the Styles list are now automatically added to the Allowed HTML tags list as well. I also updated the description text appropriately.
Thanks @BWPanda for working on this and the updated PR! I left a comment on your MR that server-side validation would work, but we have existing client-side tools to help with this exact problem of adding/removing tags from the filter list, we should use that.
https://github.com/backdrop/backdrop/pull/2777#pullrequestreview-266850766
For example dragging the Strikethrough button gives a message like this:
This also gives the opportunity for other filter modules to respond to the adding/removing of tags. This JS is a little heady though... I'll pull in your changes and then add the client-side work to my PR.
New PR up that adds the tags client-side instead of server-side, to match that of how the toolbar buttons work.
@quicksketch I tried the new PR. It partially works.
small|Small
to the Style list. And saved.So for some reason the automatic part of adding to Allowed HTML tags isn't working.
Update: there was some combo where this worked. But mostly not and when I try to recreate that combo I can't get it to work again. So that JS seems a bit fragile.
I tested this in my own sandbox. Is the 'Allowed HTML tags' field supposed to be updated as soon as I enter something in the 'Styles' field? If so, didn't work. Also didn't work when saving. Not sure if it's the PR or my understanding...
Is the 'Allowed HTML tags' field supposed to be updated as soon as I enter something in the 'Styles' field? If so, didn't work. Also didn't work when saving.
Also for me, the PR doesn't make it so that the "Allowed HTML tags" get automatically updated with values from the Styles field. @BWPanda I guess they're supposed to be added when they have been entered, at least that's how the toolbar buttons work.
Describe your issue or idea
I often want to make text in various text areas smaller. Unfortunately Backdrop's implementation of ckeditor is lacking the ability to do so :(
From stack overflow: https://stackoverflow.com/questions/24171606/ckeditor-format-tags-and-a-custom-small-tag
Search for CKEDITOR.config.format_h6 in ckeditor.js and add CKEDITOR.config.format_small={element:"small"};.
open the languagefile you want (e.g. en.js) and add "tag_small":"small text". now CKEditor supports the small tag.
Steps to reproduce (if reporting a bug)
Attempt to wrap some text in the
<small>
tag.Actual behavior (if reporting a bug)
There's no
<small>
tag.Expected behavior (if reporting a bug)
A
<small>
tag.~PR by @quicksketch: https://github.com/backdrop/backdrop/pull/2102~ ~PR by @BWPanda: https://github.com/backdrop/backdrop/pull/2777~ PR by @quicksketch: https://github.com/backdrop/backdrop/pull/2799