« Back Issue #236
  • 2014-05-02 18:39 Reported by epinapala
  • 2014-05-23 00:55 Marked fixed
Status: Marked fixed
Priority: Medium

Adding a wiki editor to Yioop.

epinapala 2014-05-02 18:39

The feature being requested is to integrate the Wiki editor into Yioop. The Wiki editor will enable the users to add HTML contents using wiki markup.

Additional information: This wiki editor is created as a part of my CS 297.

administrator 2014-05-04 09:36

Hey Eswara,

I made you a developer now. Go ahead and post your javascript.

Best, Chris

epinapala 2014-05-04 21:21

Hi Professor,

I have created a new branch from the master branch(as HEAD) - "wikieditor". Added my changes to "wikieditor" branch, committed and created a patch for all changes against master. I am attaching the patch here. Also , as the commit contains binary files("wikieditor" icon images) , I am attaching them here , just in case.

Eswara

epinapala 2014-05-05 11:57

Hi Professor,

Just figured the old patch did not have the CSS changes. Replaced the patch with some new CSS/JS changes. I will make sure the commit has up to date information before I submit a patch next time.

Rajesh

epinapala 2014-05-16 02:31

Hi Professor,

I have made the changes you suggested. For localization, I have created string constants to easily integrate with the localization. I have also added the license. I pulled the latest code and created a patch for my wiki editor changes against the master branch. I am attaching the patch as well. Please let me know If you want me to add any more changes.

Please find wikieditor_v2.patch attached.

Thanks Eswara

administrator 2014-05-16 23:06

Hi Eswara,

Thanks for the new patch. Here is what I'd like you to revise: (1) You seem to have removed the @param and @return info from all the comments. What I wanted was that you follow the same format as I use in the other Javascript files in Yioop. The previous iteration you were putting {{}} around stuff -- that's all I wanted removed. Use type names though like I do in the other JS files. (2) I want you to handle localization as is done in mix.js and classifiers.js using a global variable tl[] that get's populated in a controller in PHP land ( for testing you can have it at the top of your script.) (3) There are lots of lines over 80 characters. (4) Do not use line continuation characters. Splits string with + to concatenate. (5) File names of all files should be lower case (6) Avoid id selectors in CSS, make sure the names of selectors is unlikely to collide with future CSS imports. (7) Use \s+ rather than \s\s* in regexes (8) Do not camel case variable names like retVal, selStart, etc. Write sel_start instead. (9) Avoid using confirm and prompt. (10) Within a function, multi-line comments should not have a vertical line of *'s. Before a function it is okay.

I think that's it. Fix those formatting issues, check your patch applies without white space errors and I think it is good to go.

epinapala 2014-05-17 00:07

Hi Professor,

Thanks a lot for your feedback. I have a question regarding the comment below,

You said - "Do not camel case variable names like retVal, selStart, etc. Write "sel_start" instead." I have actually used lowercase variable names and variable names with underscores for almost all the variables. However, only at a single place, in the function - getSelection, "selectionStart" is a XUL property defined by mozilla (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Property/selectionStart) I am just trying to use the property for mozilla based browsers and for IE, as the property doesn't exist, I tried to add the similar functionality, with the same property name. This is the reason why I did not change selectionStart to selection_start in my code. Is it ok to leave this property as it is?

I will fix all other issues. I will create a patch as soon as possible. Again, thanks for your feedback.

Eswara.

epinapala 2014-05-17 08:58

HI Professor,

I added a new mechanism to display input fields for user input. Added a lot of new code to support overlay divs using pure Javascript and CSS. I completed making all other changes you mentioned except one minor change regarding camel case for a few variables . All these variables are property coded according to your coding directions. I have posted my comments in the previous message regarding the problem with changing just these ones below:

text_area.selectionStart, text_area.selectionEnd text_area.selectionText

the latest version of the editor is live at http://cyborgsavior.com/wikiedit/ , The new source code is here : http://cyborgsavior.com/wikiedit/js/wikify.js . As soon as I hear back from you regarding the problem mentioned above, I will finalize the commit and create a new patch.

Thanks, Eswara

administrator 2014-05-17 10:24

Hey Eswara,

I noticed that the H button doesn't seem to align with the other buttons. It also didn't seem to work. The other button when clicked, only insert the start and close no sample text. For example, clicking bold inserts

rather than something like

+ tl['wiki_js_bold'] + Let's use wiki_js as the prefix on strings to localize. Follow this with approximate English term.

The word Submit and Cancel need to be localizable, they are currently hard-coded in places.

wikiText => wiki-text (for an id) or wiki_text (for a variable)

Fix these issues, change elementx to element_x and make a new patch.

Otherwise, it looks fine.

Best, Chris

administrator 2014-05-17 11:58

Here are a couple more remarks: (1) make the buttons smaller by reducing white space. Get rid of green background on buttons, make light gray. (2) when insert make sure for block elements they get inserted on a new line (3) change yioop_search to search, make logo a magnifying glass

epinapala 2014-05-17 16:02

Hi Professor,

Thanks for the feedback. I have made the latest revisions you mentioned .

changes are live at http://cyborgsavior.com/wikiedit/

1) Localized the words used for buttons. 2) Added default text to add when there is no selection text. 3) I edited the buttons, and tried to clean them up. Not sure if I did that exactly as you wanted, Please take a look and let me know. 4) Changed search widget to magnifyng glass. changed yiooop_search to search in widget markup.

Thanks Eswara

administrator 2014-05-17 16:10

Eswara,

Please post your patches to this thread and I will evaluate them rather than some offsite place. It doesn't look like you have completely addressed all of the issues I mentioned, such as reducing button size, format of translate text, naming conventions on translated text, wikiText => wiki-text (for an id) or wiki_text (for a variable), etc

epinapala 2014-05-17 16:18

Hi Professor,

Sure, what I will do is , I will go through all the code once again and make sure all the revisions are addressed. Once I am done, I will create a patch and upload it here for you to review. Sorry, i was in a rush to leave out of station, so I was pushing too manu commits too quick. I will do a thourougj review of all the revisions and send a patch by end of day tomorrow, is that ok?

Rajesh

epinapala 2014-05-17 21:19

Hi Professor,

I have made the changes and verified them. I also attached a latest patch and a screenshot(http://www.seekquarry.com/mantis/file_download.php?file_id=145&type=bug) of the editor after the changes.

> For "wikiText => wiki-text (for an id) or wiki_text (for a variable)"

Only params I did not make changes to are the params from XUL reference, which I added support for in Older versions of MSIE. they are : text_area.selectionStart, text_area.selectionEnd text_area.selectionText

Except these, All the others id names and JS vars are following proper naming conventions.

> I reduced the size of the wiki buttons. Also made changes to the button colors. Please let me know If you want me to make any other styling changes.

> I changed the naming conventions on translated text. All the elements now begin with wiki_js. However, I haven't included the global translation params in the patch. Can you please let me know where can I add the new localization params?

Please let me know If you need any other changes from the latest patch.

Thanks Eswara

epinapala 2014-05-18 04:18

Hi Professor,

I haven't included the localization variables I used for testing in the patch, Where can I add these new localization variables?

Here is the list below, just for reference.

var tl = []; tl['wiki_js_search_size_small'] = "Small"; tl['wiki_js_search_size_medium'] = "Medium"; tl['wiki_js_search_size_large'] = "Large";

tl['wikify_link'] = "Link"; tl['wiki_js_search_size'] = "Size"; tl['wikify_prompt_heading'] = "Add Header row"; tl['wiki_js_example_placeholder'] = "Example"; tl['wiki_js_table_title_placeholder'] = "Title"; tl['wiki_js_formbtn_submit'] = "Submit"; tl['wiki_js_formbtn_cancel'] = "Cancel"; tl['wiki_js_bold'] = "Bold text"; tl['wiki_js_italic'] = "Italic text" tl['wiki_js_underline'] = "Underlined text"; tl['wiki_js_strike'] = "Striked text"; tl['wiki_js_heading'] = "Heading"; tl['wiki_js_bullet'] = "Bulleted list item"; tl['wiki_js_enum'] = "Numbered list item"; tl['wiki_js_nowiki'] = "Insert non-formatted text here";

//prompts tl['wiki_js_prompt_search_size'] = "Select Yioop Search widget size.(S,M or L) " tl['wikify_prompt_for_table_cols'] = "Enter the number of Cols: "; tl['wikify_prompt_for_table_rows'] = "Enter the number of Rows: "; tl['wikify_enter_link_title_placeholder'] = "Enter Link title"; tl['wikify_enter_link_placeholder'] =

       "Enter link target: ";

tl['wikify_link_title'] = "Enter Link Title: ";

Thanks Eswara

administrator 2014-05-18 08:13

I've messed around with this some, but eventually back it out. Here is some feedback.

(1) Make a non-anonymous function that takes as argument the id that you would like to be editor-ized. Leave it to the user to add an onload event. (2) Having two code blocks for rendering the editor is not maintainable. Pick either the old IE one or the newer one. In my experience, using innerHTML tends to be the most cross-platform even if it looks ugly. (3) The newer one is in some sense more awkward because as a quick way to allow non Javascript support I tried coding: <div id='wiki-editor'> <textarea>some content</textarea> </div> Rather than replace the above textarea it instead appends. (4) I think the ideal way to include your editor might be something like: <textarea id="wiki-editor">some content</textarea> It should first grab "some content", then replace the textarea node in the DOM with your stuff, replace the contents of the new textarea with "some content". (5) You can't assume the placement of your editor on the page. If your editor is at the bottom of a long page, the pop-ups seem to show up in the wrong place. (6) You can't assume in your code that there is only one editor on the page. The way you are currently using document.getElementById everywhere you do. (7) In Yioop PHP-land you should make a Helper class (see under views/helpers) that inserts a wiki-editor into a page. It should add to $data['SCRIPT'] the settings of the variable tl for the current locale. If you look at the editMix method of controllers/social_component.php, you can see an example of this kind of code. (8) You are getting closer to done on this patch because my comments are becoming more substantive.

epinapala 2014-05-18 12:27

Hi Professor,

Thanks for your comments. I too felt it would be difficult to maintain code with two approaches. I will try to complete the changes mentioned above and testing as soon as I can before mid next week. Is it ok ? If there is a rough eta you are expecting me to push the patch, please let me know.

Thanks Eswara

epinapala 2014-05-19 18:05

Hi Professor,

I started working on the changes and I have one question regarding your comment below -

"I think the ideal way to include your editor might be something like: <textarea id="wiki-editor">some content</textarea> It should first grab "some content", then replace the textarea node in the DOM with your stuff, replace the contents of the new textarea with "some content"."

Question : When I replace the textarea node in the dom with my textarea, the attributes like class, name etc would be removed. Should I store all the other attributes(other than id, like class, etc.) and add them to the target textarea before I replace it?

Thanks, Eswara

administrator 2014-05-19 18:56

Yes.

epinapala 2014-05-20 15:13

Hi Professor,

I completed making changes to my code, Now I am working on integrating the editor with Yioop. I need some help from you to integrate the editor in the yioop system.

I have created a Helper class(wikijs_helper) to insert the JS code required to "editorize" a textarea identified with it's id. When I tried to all the render method in the helper does is, set the $data['SCRIPT'] & $data['INCLUDE_SCRIPTS'] variables. This is exactly similar to what editMix method does.

I am trying to integrate the editor in wiki_view class, however the $data['SCRIPT'] & $data['INCLUDE_SCRIPTS'] variables are not being set for some reason. from debugging I confirmed that the variables are being set in the helper class for sure, but they are not being rendered in the WikiView::renderEditPageForm() method.

Can I create a patch with my changes so far, so that you can take a look? I will write up the detailed change log while creating the patch.

Eswara.

administrator 2014-05-20 16:05

Sure post a patch. Rename the helper class wikieditor_helper.

epinapala 2014-05-20 16:29

Hi Professor,

I am attaching another patch with changes requested. change log below:

> Made two non-anonymous fucntions editorize(textarea_id) and editorizeAll() > The id of the textarea to be editorized can be passed to editorize function, If all the textareas need to be editorized, user can call editoizeAll(). > Now the editor is rendered using .innerHTML technique instead of DOM amnipulation. > Now the popups are modified to act similar to how MediaWiki does user input overlay. The input form overlay appears at the center of the page. Please let me know your thoughts. > Assumptions that there will be only one editor are not done anymore. As many textareas as user likes, can be editorized. The id of each textarea is used to track button rendering, button actions, user input etc. > Tried creating the Helper class. Need some help from Professor to completely integrate the localization and javascript. For now, added temporary changes in web_layout for demo purposes. Will make more changes after pointers from professor.

Eswara

epinapala 2014-05-22 11:46

Hi Professor,

Just wanted to check If you got a chance to look at my latest patch. Please let me know, if you need any other changes in the code.

Thanks Eswara

administrator 2014-05-22 12:41

Hey Eswara,

I applied your patch. (Not live yet). I will tweak it to what I want probably by late this afternoon. Here are some comments on your code for the next time you need to make a patch: (1) CSS Declaration blocks should have the property : value pairs ordered by property. (2) You still had a lot of lines over 80 characters. These should occur no where in any patch. (3) Every property value pair should be on its own line. (4) Favor the short hand color value like #FFF over #FFFFFF. Avoid colors like #E5E5E5, #EEE is close enough. After I am done editing I will tack on some more comments

epinapala 2014-05-22 12:57

Hi Professor,

Thanks for your feedback. Regarding the 80 characters, Is it the CSS file having more than 80 characters? I made sure the JavaScript has all lines under 80. Sorry, I haven't followed the same rule for CSS. I can make changes to the CSS file and create another patch If you want.

Thanks Rajesh

administrator 2014-05-23 00:55

Hey Eswara,

That took way longer than I thought it would. In any case, your code after extensive editing is now live. Do a git diff between your version and mine to see what I changed. I tested my code on the latest Firefox, Safari, and Internet Explorer and it seems to work. Except for the selection stuff all the code was pretty vanilla so should work back at least a few years on all the major browsers.

Best, Chris

administrator 2014-05-23 00:55

Fixed by c6cdfc6

X