CVS edit link for tronathan

I would like to submit a module called tinymce_linkpicker.

It provides a bridge between the wysiwyg editor tinymce and drupal. It works by adding a button to tinymce (manual process to install, same process as img_assist) and connecting the target of a popup window with a specific view that display links which contain javascript that inserts the target of the link (along with the title of the link in an A HREF) into the editor.

This provides a feature that is competitive with Google Sites and one of the shortcomings of Drupal when compared to Google Sites.

Currently the module displays a view which the user can edit to customize the view.

CommentFileSizeAuthor
#7 tinymce_node_picker6.x-0.10-dev.zip27.95 KBtronathan
#5 tinymce_node_picker6.x-0.9-dev.zip28.41 KBtronathan
#1 tinymce_node_picker6.x-0.8-dev.zip26.67 KBtronathan

Comments

tronathan’s picture

tronathan commented
Status: Postponed (maintainer needs more info) » Needs review
Issue tags: +tinymce, +WYSIWYG API
StatusFileSize
new tinymce_node_picker6.x-0.8-dev.zip26.67 KB

Please find the attached plugin for Drupal and TinyMCE which provides a bridge to create links to nodes from the tinymce toolbar. This is similar to the functionality available in Google Sites.

tronathan’s picture

tronathan commented

Updating status to 'needs review'

ajk’s picture

ajk commented
Status: Needs review » Needs work

This is just silly for tinymce_node_picker.module :-

include_once('tinymce_node_picker.features.inc');

... and nothing else. Why not just take the code from the .inc file and put it in your module and save the server having to load another file unnecessarily?

And then in tinymce_node_picker.features.inc :-


/**
 * Implementation of hook_views_default_views().
 */
function tinymce_node_picker_views_default_views() {
  module_load_include('inc', 'tinymce_node_picker', 'tinymce_node_picker.defaults');
  return _tinymce_node_picker_views_default_views();
}

... and that's all that's in that, loading yet another file! And all that does is define an implementation of hook_views_default_views().

Why is all this "include" going on?

tronathan’s picture

tronathan commented
Status: Needs work » Needs review

The contents of the module were generated by the Features module. I'd like to continue using the output from the Features module for consistency and to make updates easier. However, if the code does not meet some standards, I can refactor it.

I realize that stylistically this might not be what you are expecting, but unless the performance hit is significant, I would like to use this structure which is consistent with how features generates code.

Please advise if the module still needs work in light of these thoughts.

tronathan’s picture

tronathan commented
StatusFileSize
new tinymce_node_picker6.x-0.9-dev.zip28.41 KB

I reviewed the contents of the files and agree, the includes are not necessary and trivial to remove. I removed the two .inc files, now the views_default_views() function is in the .module file directly.

Please review and approve.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work 
  1. /**
 * $Id: editor_plugin_src.js 201 2007-02-12 15:56:56Z spocke $
 *
 * @author Moxiecode
 * @copyright Copyright � 2004-2008, Moxiecode Systems AB, All rights reserved.
 */

    Files that are not licensed under GPL Licensed cannot be hosted on Drupal.org CVS.
    The reported comment contains a strange character that I was not able to read (between "Copyright", and "2004"). 

  2. 		getInfo : function() {
			return {
				longname : 'Druapl Link Picker Plugin',  // <-- It should be Drupal
				author : 'Jonathan Yankovich',

    Replace any tab characters used to indent the code with two spaces.
    You declare you are the author, but the code is copyright by Moxiecode (which is reported to be the author too, in the first comment).

  3. The files contained in the directory Theme have been committed in CVS by somebody else.
tronathan’s picture

tronathan commented
StatusFileSize
new tinymce_node_picker6.x-0.10-dev.zip27.95 KB

1. The plugin was created using the provided template, which is the process that moxicode recommends for developing tinymce plugins. The approved img_assist module uses the same technique and plugin template file. I removed the copyright statement, which is the same thing that the already-approved img_assist plugin for tinymce does. Here is the cvs source for img_assist which uses the same technique:

http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/img_assist/...

Note that the editor itself is not being included, only a plugin javascript which uses the api template developed by moxicode, designed for integration with tinymce.

Please advise if I should take a different approach to this.

2. Spelling error for "drupal" fixed, tabs replaced with (2) spaces in editor_plugin.js and tinymce_linkpicker module, as well as 2 theme templates.

3. The files in the theme directory are copies of default page templates from the acquia_marina theme, which have had extraneous theme information (blocks, variables) removed. The original file is in CVS at:

http://cvs.drupal.org/viewvc.py/drupal/contributions/themes/acquia_marin...

I removed the following comments from these files:

// $Id: page.tpl.php,v 1.1.2.6 2009/05/13 09:26:06 jwolf Exp $

Let me know if there's a better way to handle a situation where you edit someone else's template in this manor.

Please find the attached version 0.10 for review.

ajk’s picture

ajk commented
Status: Needs work » Needs review
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

I would rather use a more neutral template. What does happen if the site is using a theme that is someway not compatible with the one used by the module?

tronathan’s picture

tronathan commented

If you look at the included template, it is very basic and only uses standard variables along with the content block. All of the containers (content-inner, etc) are intact and will take on the style of whatever theme the user is using. I dont think the theme files need to be changed as they -are- compatible with standard/existing themes.

Please provide more specific feedback or approve the module.

-Jonathan

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Fixed

The README.txt reports a wrong link for the WYSIWYG module, and the LICENSE.txt must not be included in the repository because it's already added by the packaging script.
For the rest, the module is good.

tronathan’s picture

tronathan commented
Status: Fixed » Closed (fixed)

1) Readme updated

2) License file removed

Thank you for the approval. The project has been created at:

http://drupal.org/project/tinymce_node_picker

Additional development information will be available there. Closing this issue.

ajk’s picture

ajk commented
Status: Closed (fixed) » Fixed

Once an issue is "fixed" please leave it at that. Drupal.org automatically closes fixed issues after two weeks.

Status: Fixed » Closed (fixed)
Issue tags: -tinymce, -WYSIWYG API

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

Issue tags: +tinymce, +WYSIWYG API

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: Miscellaneous » new project application
Assigned: Unassigned » avpaderno
Issue summary: View changes
Status: Closed (fixed) » Fixed
Issue tags: -tinymce, -WYSIWYG API

Status: Fixed » Closed (fixed)

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