[14.0][ADD] web_widget_html_markdown#7
Conversation
thomaspaulb
left a comment
There was a problem hiding this comment.
Generally OK, could use a cleanup pass, we can do it together in a sprint.
| 2) Write some markdown and then add some HTML. ( will cause loss of HTML if you edit markdown again) | ||
| 3) Only markdown. | ||
| - RE-editing pure markdown content is LOSSLESS because we preserve the markdown content in a special tag. | ||
| - Every switch to markdown will cause destruction of HTML. Html Can Be added after writing Markdown part but will always be deleted. |
There was a problem hiding this comment.
I would change this a little bit:
This module implements a Markdown editor on Html fields, in contract to web_widget_text_markdown, which implements it on Text fields. In readonly mode, the widget displays HTML, but when editing, the widget offers you an option to edit in Markdown or in HTML. If you edit markdown, it will save as the rendered HTML, but with the source Markdown embedded inside a <script> tag. When editing again, it will show you the Markdown source. If you edit HTML, you will lose the Markdown and the content will just behave as a regular HTML field with an HTML widget.
The benefit of this module over web_widget_text_html is that it allows markdown-editing of HTML fields such as for example the mail.message body for the chatter, or HTML fields that end up in printed reports such as the Quotation description. Such fields cannot be converted to Text fields because it will cause problems for their functionality. But with this widget, they can still be edited as Markdown.
A difficulty with this module is that once you start editing a field as HTML, whether with this Markdown widget in HTML mode, or through a view that has the normal HTML editor, you will lose the Markdown source and you will have to keep editing the field as HTML, or do a backconversion to Markdown. This backconversion is always lossy to a certain extent and we did not bother implementing it - but it's something for the roadmap.
There was a problem hiding this comment.
I like. shorter. Done. I used your text, but i left the two titles.
| @@ -0,0 +1 @@ | |||
| * HTML Will be lost when passing from Html to markdown | |||
There was a problem hiding this comment.
- Implement conversion from HTML to Markdown, either through a JS or a Python module, instead of currently just wiping the content of the field.
There was a problem hiding this comment.
Done but slightly different, now that we changed stuff:
- Implement conversion from HTML to Markdown, either through a JS or a Python module, instead of not allowing it, or allowing it only after HTML is emptied.
| ... | ||
|
|
||
| The widget is called unidiectional because itt is used on a Html field to edit HTML | ||
| or Markdown, but the conversion is unidirectional, ONLY from Markdown to HTML. |
There was a problem hiding this comment.
This part I think can go, the DESCRIPTION explains it enough
| or Markdown, but the conversion is unidirectional, ONLY from Markdown to HTML. | ||
|
|
||
| This will replace the default Html widget by the new Markdown/HTML widget and | ||
| allow the field to be edited as Markdown or HTML, depending on the user's choice. |
There was a problem hiding this comment.
This part is good, and then below we can explain the new options that will be added.
| .replace(/\<\/p\>/g,'') | ||
| .replace(/\<\/br\>/g,'') | ||
| .replace(/\<br\>/g,'') | ||
| .replace(/ /g,'') |
There was a problem hiding this comment.
Doesnt the isSet function already do this?
There was a problem hiding this comment.
This works on the current value, and isSet works on the last-saved value.
|
|
||
|
|
||
|
|
||
| var FieldHtmlMarkDown = basic_fields.DebouncedField.extend(TranslatableFieldMixin, { |
There was a problem hiding this comment.
We should, I think, spend some together-time still on trying to make this widget inherit directly from HtmlField. I shot down that idea earlier but we duplicate a lot of code here, so it's worth another shot.
| // this will use marked | ||
| var render_val = self._getHtmlValue(e.getContent()); | ||
| return render_val; | ||
| }, |
There was a problem hiding this comment.
Currently the dropzone.js functionality, added in the 11.0 version of the text widget module, does not work. We can re-enable it.
| this.$el.prepend(md_s); | ||
| // | ||
| this.$el.find('input').addClass('mk_html_switch'); | ||
| this.$el.find('input').change([self], self._switch_markdown_html); |
There was a problem hiding this comment.
I think it's best practice to make a separate sub-widget out of this and then instantiate it with:
this.sub_widget = new SubWidgetClass(this);
this.$el.prepend(this.sub_widget);
And then do the events in the Odoo events: way, let them bubble up to here via messages, and then catch them here.
There was a problem hiding this comment.
@thomaspaulb Question: Are we sure? All other issues are done. I see the radiobutton+the editors as one widget.
The radiobutton+information_area as a separate widget would make sense if it was reusable elsewhere, this part of the widget is used once and makes sense only in conjunction with the rest. so I don't see why we should separate this.
After the cleanup, the widget is 200lines, i have refactored the views in XML, so if your concern is length of code it should not be a problem.
I am all for modularity, but this sub-widget has not use anywhere else.
| var has_md_source_tag = self._html_has_md_source_tag(); | ||
| if (!is_empty && !has_md_source_tag ) { | ||
| // THIS SHOULD NEVER APPEAR, WE HAVE HIDDEN THE RADIOBUTTON IN ALL | ||
| // POSSIBLE CIRCUMSTANCES, KEEPING IT FOR TESTING PURPOSES. |
There was a problem hiding this comment.
Are we now keeping the radiobutton or not?
There was a problem hiding this comment.
Gio says we can clean it up.
|
@thomaspaulb All errors found. Tested, works perfect
ONLY ONE SMALL PROBLEM: SHould be quite easy to extend though, i am surprised it is not so in HTML widget. ( confirmed, HTML widget did not see those clipboard, mouse and actions as "onchange" events. REST WORKS PERFECT |
4bd0744 to
dbfc425
Compare
web_widget_html_markdown/README.rst
Outdated
| @@ -0,0 +1,131 @@ | |||
| ======================================== | |||
| Web Widget Html Markdown Unidirectional | |||
| ======================================== | |||
There was a problem hiding this comment.
reset this to an empty file, it will be autogenerated anyway
There was a problem hiding this comment.
This title is fetched from manifest. I gave it this name , in the hopes of a 2-way version. But i will remove.
If we make a 2-way version it will be a replacement. cleaning up.
| @@ -0,0 +1,27 @@ | |||
| # Copyright (C) 2014 Sudokeys (<http://www.sudokeys.com>) | |||
| # Copyright (C) 2017 Komit (<http://www.komit-consulting.com>) | |||
| @@ -0,0 +1 @@ | |||
| * Therp B.V. https://therp.nl | |||
| " <div id='radio_container'> <input id='markdown' type='radio' name='mk_html_switcher' value='markdown'>"+ | ||
| " <label id='markdown_label' for='markdown'>Markdown</label> " + | ||
| "<input type='radio' id='html' name='mk_html_switcher' value='html' checked >" + | ||
| " <label id='html_lable' for='html'>HTML</label></div><div id='md_infoarea'></div>") |
There was a problem hiding this comment.
Make this a template
| var layoutInfo = $.summernote.core.dom.makeLayoutInfo(this.wysiwyg.$editor); | ||
| $.summernote.pluginEvents.codeview(undefined, undefined, layoutInfo, false); | ||
| } | ||
| if (self.mode == 'edit' && self.$is_markdown) { |
| // we use recursion (max1) is_markdown is reset | ||
| return self.commitChanges() | ||
| }); | ||
| } else if (self.mode == 'edit') { |
There was a problem hiding this comment.
But what if mode is not edit? Shouldnt we still call super?
There was a problem hiding this comment.
No/ this is taken from source HTML. if we are not in edit mode commit changes shouldnt be ran.
We have replicated 5 lines of code because we had to add forceChange:true.
I explain in a brief comment.
Not done.
| var has_md_source_tag = self._html_has_md_source_tag(); | ||
| if (!is_empty && !has_md_source_tag ) { | ||
| // THIS SHOULD NEVER APPEAR, WE HAVE HIDDEN THE RADIOBUTTON IN ALL | ||
| // POSSIBLE CIRCUMSTANCES, KEEPING IT FOR TESTING PURPOSES. |
There was a problem hiding this comment.
Gio says we can clean it up.
| .replace(/\<\/p\>/g,'') | ||
| .replace(/\<\/br\>/g,'') | ||
| .replace(/\<br\>/g,'') | ||
| .replace(/ /g,'') |
There was a problem hiding this comment.
This works on the current value, and isSet works on the last-saved value.
a77dbc6 to
7b126be
Compare
|
@thomaspaulb all done. one commit. Just one question. |
d35c428 to
0cee218
Compare
|
@thomaspaulb added all the options, and verified for no regressions. While testing, added a useful badge that tells user in readonly mode if content was saved in pure HTML or with Markdown tag. |
0de1384 to
a4d10e7
Compare
|
@thomaspaulb working and testable. Had some troubles , solved. |
c552826 to
cbb493a
Compare
b762848 to
c984871
Compare
6278a15 to
b5a08d7
Compare


No description provided.