Requested by webchick.

Comments

sun’s picture

Status: Active » Needs review
StatusFileSize
new7.01 KB
new545 bytes

Well, here we go.

Unfortunately, this requires a patch to the editor library itself, which I'm attaching, too.

Copy of my mail to Jay:

Hi Jay,

I am the maintainer of the Wysiwyg API project for Drupal, the new home of the markItUp integration for Drupal: http://drupal.org/project/wysiwyg

We are currently working on an improved implementation of markItUp in http://drupal.org/node/352295. Everything worked well so far, but we were unable to pass a custom markupSet as an argument to the editor.

The cause is that in the function dropMenus(), the passed in 'markupSet' variable is not properly iterated over. In Drupal, we are not defining the toolbar set statically, but via dynamically defined settings that get passed to the editor instead - like so:

jQuery.extend(Drupal.settings, {
  basePath: "/",
  wysiwyg: {
    configs: {
      markitup: {
        format1: {
          root: "/sites/default/modules/wysiwyg/markitup/",
          nameSpace: "default",
          markupSet: [
            { name: "Bold", className: "markitup-bold", key: "B", openWith: "(!(\x3cstrong\x3e|!|\x3cb\x3e)!)", closeWith: "(!(\x3c/strong\x3e|!|\x3c/b\x3e)!)" },
            { name: "Italic", className: "markitup-italic", key: "I", openWith: "(!(\x3cem\x3e|!|\x3ci\x3e)!)", closeWith: "(!(\x3c/em\x3e|!|\x3c/i\x3e)!)" },
            { name: "Strike-through", className: "markitup-stroke", key: "S", openWith: "\x3cdel\x3e", closeWith: "\x3c/del\x3e" }
          ]
        }
      }
    }
  }
});

The attached patch file is against 1.1.4 and changes just the way the dropMenus() function iterates over the passed in markupSet.

It would be great if this could be part of the next release. As we will be forced to depend the integration on this bugfix, we will require at least version 1.1.5 to prevent users from using a non-working script.

webchick’s picture

Status: Needs review » Needs work

Wow!! 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:

+        'bold' => t('Bold'), 'italic' => t('Italic'),
sun’s picture

Status: Needs work » Needs review
StatusFileSize
new7.54 KB

- .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.

sun’s picture

Status: Needs review » Fixed
StatusFileSize
new7.55 KB

Committed to all branches.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.