Updated: Comment #0
Now that we have a WYSIWYG editor in core, it makes little sense to have the "filter guidelines" at the bottom.
(Bojhan and I decided to do this as a sort of "phase 1 of #2004284: Integrate "text formats selector" into CKEditor toolbar".)
Remove the filter guidelines for every text format for which a text editor is enabled.
Fix the CSS.
No more filter guidelines for a text format if that text format has a text editor enabled.
None.
#2004284: Integrate "text formats selector" into CKEditor toolbar
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | filter_guidelines-2098071-20.patch | 3.22 KB | wim leers |
| #20 | interdiff.txt | 504 bytes | wim leers |
| #20 | Screen Shot 2013-10-28 at 18.49.56.png | 20 KB | wim leers |
| #16 | filter_guidelines-2098071-15.patch | 3.17 KB | wim leers |
| #16 | interdiff.txt | 755 bytes | wim leers |
Comments
Comment #1
wim leersHere's the patch that does the PHP side of things. Things look broken now when the filter guidelines are absent; the CSS must be improved to handle that; Bojhan said he'd like to work on that, so assigning to him :)
Comment #2
Bojhan commentedHere you go, I tried to do a interdiff but I failed somewhere and got in a lot of stuff that wasn't part of this patch.
Comment #4
Bojhan commentedNoooo! I changed one piece of text though :)
Comment #5
Bojhan commented#2: spacing.textformat.patch queued for re-testing.
Comment #6
Bojhan commentedOk, some more alignment issues.
Comment #7
Bojhan commentedComment #8
BarisW commentedThe patch works fine, but don't we need the filter tips if a user decides to use a non-wysiwyg filter? Like Restricted HTML?
Before the patch (WYSIWYG):
After the patch (WYSIWYG):
After the patch - wrong? (NO WYSIWYG):
Comment #9
wim leers#8: oh, yes, definitely. Your screenshots show that the proposed resolution
is not being met.
I'll fix that.
Comment #10
wim leersAforementioned problem fixed.
Also: I noticed that Bojhan's CSS changes were quite invasive and significantly changed the positioning. This updated patch makes far less CSS changes and keeps everything in the exact same place.
Comment #11
wim leers.
Comment #12
swentel commentedunsetting things is not really friendly is it, and we don't tend todo that afaik. Better to use #access => FALSE no ?
Comment #13
Bojhan commentedCould this get screens?
Comment #14
Bojhan commentedI noticed the margins are changed. In the screenshots of #6 show that the "Text format" label aligns with the WYSIWYG buttons and the same spacing is applied to the help icon + label. That seems to be lost now.
The rest looks great :) The thing I am wondering, if there should be a little ease when you flip between having no WYSIWYG and having a WYSIWYG its now all very sudden.
Comment #15
wim leers#12: good point. Done.
#13: Heh — that was intentional: the margins are not changed in my patch, they were changed in your patch. I don't think it's a great idea to make the text format
<select>align with CKEditor's buttons — what if you use a different style on CKEditor? What if you use another text editor than CKEditor? In any case, changing alignment is completely unrelated to the goal of this issue, so if you want to change that, please do it in a different issue. Let's get this in now :)Comment #16
wim leersUgh forgot to attach the new patch in response to #12.
Comment #18
wim leers#16: filter_guidelines-2098071-15.patch queued for re-testing.
Comment #19
Bojhan commentedThe margins look bad now, I'd like to fix it. I think the change that I made looks fine with and/or without CKEditor. Unless you feel really passionate about the few px margin change, can we please just fix it here. I'd like to introduce a design change that doesn't require followups.
Comment #20
wim leersAlright then. Please review!
Comment #21
Bojhan commentedAwesome, thanks! This looks soo much cleaner.
Small followup would be to fix the ? icon to be retina ready.
Comment #22
wim leers#21: Sure, go ahead :) That's entirely unrelated to this though.
Comment #22.0
wim leersUpdated issue summary.
Comment #23
wim leers20: filter_guidelines-2098071-20.patch queued for re-testing.
Comment #24
webchickThis might be my favourite patch of all time. ;)
Committed and pushed to 8.x. YEAH! Down with confusing, verbose, and unnecessary crap!
Comment #25
wim leersComment #26
nod_