Make dependency on TinyMCE optional and integrate Imperavi #16

Closed
wants to merge 2 commits into
from

Projects

None yet

2 participants

@jleclanche

This commit achieves two things:

  • It removes the forced dependency on TinyMCE, but keeps the integration optional
  • If django-imperavi (another, more lightweight rich text editor for django) is found, it integrates with that instead of tinymce. It's also optional.
jleclanche added some commits Aug 12, 2012
@jleclanche jleclanche Make setup.py executable bf0299a
@jleclanche jleclanche Make the dependency on TinyMCE optional
Also integrate Imperavi instead of TinyMCE if it's found
1374782
@dokterbob
Owner

The general idea of not depending on a single text-editor sounds good, but it needs some documentation - users need to be able to find out how to enable either one without having to dive into the code.

Perhaps also an explicit setting would be (very) nice, so there is no built-in preference for a particular editor. Maybe something like NEWSLETTER_RICHTEXT_FIELD?

Also, thanks for the whitespace removal. It was much needed. If you'd fire a pull request for just that I'd merge it straight away.

@jleclanche

Currently, it's on imports: If we have imperavi, it integrates that. Else if we have tinymce, it integrates that. Otherwise it leaves it as a text field. If you're ok with that, I can update the readme to say just that.

The only case where a setting would be appropriate would be if the user has more than one text editor installed. I can see that happening, but if they do I don't think they care about which editor is used then... imho, keeping it simple is better here.

With regards to the whitespace removal, I can fire up a commit for all of it, to me it's just a ctrl+s.

@dokterbob
Owner

Thing is, as long as I haven't had the time to look at this other editor - I don't think it's wise to have it be set as a 'default'.

Also, some people might operate django-newsletter in a virtualenv with some editor installed globally - in which case the current behaviour is rather haphazard (ie. to switch editor one might have to uninstall a package globally).

Lastly, having an explicit setting makes sense as a form of 'documentation' - a settings file would be the first place a lot of people might look to change editors. Also, imperavi might not be the best editor in the long term (who knows?) - imagine having to add another conditional import to the current scheme. ^^

Hence, I would really really like to see some kind of setting for this. You can, and should, keep it simple - ain't nothing like the KISS principle. But profound 'automatic' and unconfigurable change in behaviour depending on installed packages just seems unwise to me.

About the whitespace patch: please do. Most of this stuff was written some time ago and I haven't had the time to clean whitespaces up since.

@jleclanche

Fair enough, I'll add a setting. The whitespace patch will come first to make it simple for git ;) Give me a few minutes though.

@dokterbob
Owner

@Adys Thanks! \o/

Meanwhile, I've created a different issue for this specific feature. Also, having checked out imperavi I have to say: it rocks! Hence, it'd be nice if you could make it the default in your patch - and perhaps emit a log warning if the import could not be found. Also, thanks for the tip. It'll be in my standard toolkit pretty soon, I think. :)

Sort-of a recommendation to users to install it - as using django-newsletter without a nice texteditor is kind of like icecream without the cherry on top. ;)

@dokterbob
Owner

Closed while awaiting whitespace-patch and #19.

@dokterbob dokterbob closed this Aug 13, 2012
@dokterbob dokterbob referenced this pull request in dokterbob/django-portfolio Mar 18, 2013
Closed

tinymce integration / template tags #2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment