Closed (fixed)
Project:
Wysiwyg
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
29 Dec 2008 at 23:16 UTC
Updated:
20 Jan 2009 at 01:50 UTC
Jump to comment: Most recent file
Comments
Comment #1
sunWell, here we go.
Unfortunately, this requires a patch to the editor library itself, which I'm attaching, too.
Copy of my mail to Jay:
Comment #2
webchickWow!! Thanks, sun!! :D
I tested this out and it worked great for basic filtered html stuff. I added some "nice to have" items at http://drupal.org/node/313497#comment-1175270.
Now if I were to give this a proper review, I'd point out:
* There's a hunk in there for .cvsignore.
* editors/markitup.inc is missing a @file declaration. :P
* I would default the skin to 'simple' instead, as it's much more likely to not clash horribly with a given theme.
* That's strange that $default_buttons is defined in the .inc file; can this not be directed to markitup's enabled set to define? One of markitup's strengths is that you can swap out "sets" so that the buttons can generate HTML or Markdown or BBCode or what have you; hard-coding this seems to be a step backward.
* Same thing with the CSS file.
* I would put this on two lines like the rest:
Comment #3
sun- .cvsignore: Used to protect CVS from accidentally committing an (external) editor library. So this is actually required.
- markItUp's "sets" define both the toolbar AND the buttons, and on top of this, the buttons define the actual markup that is inserted. This is a special case, for which we have to find a (yet another) solution. In this case, I could think of two possibilities: a) We remove the capability to configure individual buttons and allow to configure hard-coded sets instead (#313497), or b) we define our own sets and languages, and configure the buttons according to the configured language (#313497 again). Thinking future-proof, we can already forget about a), since Drupal modules in contrib have to be able to define additional toolbar buttons (f.e. to insert an image [macro tag], like Image Assist does). Thus, we need to define our own sets, and also load our own CSS file to load the corresponding button images.
- The coding-style for button definitions actually follows a consistent structure - they are grouped by functionality/category. Admittedly, it is not very visible in markup.inc. ;) However, as long as #277954: Allow to sort editor buttons is not fixed, I'm trying to keep all editor integration files as comparable as possible for maintenance reasons.
Comment #4
sunCommitted to all branches.