Support from Acquia helps fund testing for Drupal Acquia logo

Comments

loominade’s picture

Status: Active » Needs review
FileSize
2.06 KB

this is my first wysiwyg contribution. Please review!

yannickoo’s picture

I attached the patch which creates both files so you don't have do to copy and paste stuff.

yannickoo’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
80.77 KB
6.35 KB

So I updated the patch so that the theme is working and I removed three unnecessary hooks and added the wysiwyg_hook_install_note function. I tested with the latest WYSIWYG version and it works fine. I also attached a fancy screenshot so you can better imagine what this patch does.

Screen Shot 2012-08-15 at 19.08.00.png

yannickoo’s picture

Added three missing hooks.

loominade’s picture

Oh, by the way: a skin called 'Bartik' has just been added to EpicEditor

erikwebb’s picture

Status: Reviewed & tested by the community » Needs review

I've started a separate project to implement this - https://drupal.org/sandbox/erikwebb/1674176

EpicEditor doesn't really follow the same pattern as other editors, because it's actually saving what you're typing rather than what's being created in the background. Either way, maybe my code can help some.

yannickoo’s picture

Cool erikwebb but what do you mean by "it's actually saving what you're typing rather than what's being created in the background". The patch works fine for WYSIWYG module and to have this as a seperate project is worse than have it as an WYSIWYG editor.

Why do you set the status to needs review?

erikwebb’s picture

I'll try and review it, but figured someone else should mark RTBC since you committed the patch yourself.

sun’s picture

FileSize
4.53 KB

Sorry, DrupalCon Munich kept me busy for a while.

Wysiwyg module supports markItUp and others already, which are equally Markdown editors. So this is perfectly fine.

I wanted to commit this, but hrm... the code doesn't really follow the editor implementations. :(

So I sat down and corrected it - blindly; i.e., I did not try whether the new code works. See attached patch.

I've committed this patch to 7.x-2.x, so patching and working on EpicEditor support should get easier from here.

loominade’s picture

@erikwebb: oh, thats a misconception. I wrote the code and uploaded the files I added, because I didn't know how to put new files in a patch. yannickoo put them in a patch file and reviewed it

Nevertheless a some more feedback would be nice since this is the first time I ever worked with wysiwyg

sun’s picture

Status: Needs review » Needs work
TravisCarden’s picture

Status: Needs work » Needs review
FileSize
476 bytes

The editor.unload() code in Drupal.wysiwyg.editor.detach.epiceditor needs to test for a trigger value of "unload" or it'll remove the editor any time the form is affected via AJAX, such as when you upload a file to a file field. Here's a patch.

Note: I learned this lesson in my own first experience patching WYSIWYG, to add Ace support. I also figured out how to let the user select an arbitrary theme and other non-standard config options. Feel free to rip off any of the code, if you like: #1377948-18: Add support for Ace (Ajax.org Cloud9 Editor). :)

caponey’s picture

Epic Editor is so nice. I can see this as becoming the future of editors, especially for Mobile first, responsive websites.

@TravisCarden, that patch fixed an issue I was having, thank you. Anyone else having a conflict with the Overlay module? Basically, the patch Travis made didn't work until I disabled Overlay. Another issue I am having is that whenever I switch between formats, it adds an editor wrapper every time, pushing the select list down the height of the editor.

mgifford’s picture

The link from /admin/config/content/wysiwyg goes to a broken link:
http://oscargodson.github.io/EpicEditor/docs/downloads/EpicEditor-v0.1.1...

yannickoo’s picture

yannickoo’s picture

@#12: Could you please create a new issue for that?

TravisCarden’s picture

Re @yannickoo, #16: I suppose. :) #1988128: EpicEditor disappears on any AJAX form operation Do we close this issue as fixed now?

yannickoo’s picture

Status: Needs review » Fixed

Yes. I think it is better to create follow-up issues so that it is easier to seperate the issues :)

Status: Fixed » Closed (fixed)

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