-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
14.0 web widget html markdown #6
base: 14.0
Are you sure you want to change the base?
14.0 web widget html markdown #6
Conversation
NOTE: continuation of #4, see comments there also. |
web_widget_html_markdown/static/src/js/web_widget_html_markdown.js
Outdated
Show resolved
Hide resolved
'change .mk_html_switch' : '_switch_markdown_html', | ||
}), | ||
*/ | ||
var FieldHtmlMarkDown = basic_fields.DebouncedField.extend(TranslatableFieldMixin, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you've basically copied a lot of code from the original HTML widget of Odoo. And we're also using it, of course, when the user switches to Html.
To avoid duplication of code, we basically have two options:
-
Don't inherit from
DebouncedField
, but directly from HtmlField instead and then override its workings. I remember I specifically asked you to inherit fromDebouncedField
and it was because I saw some stuff in the Html widget's code that looked incompatible with what we wanted to do, but that was before we decided to offer live switching between HTML and Markdown. The situation might look different now - what's your thought on this? -
Instead of creating a new widget, we just enhance the existing HTML widget so that ALL Html fields in the system can now be edited with Markdown. Seems on one hand very useful, but also a bit radical - at least user should be able to switch off that behaviour in order to have a remedy for any bugs that our enhancements might cause in his particular incarnation of Odoo.
I'm not bothered for now but let's keep this as an open discussion point for when we polish this module for OCA.
web_widget_html_markdown/static/src/js/web_widget_html_markdown.js
Outdated
Show resolved
Hide resolved
web_widget_html_markdown/static/src/js/web_widget_html_markdown.js
Outdated
Show resolved
Hide resolved
code replication: We had decided that inheriting from Htmlfield was not possible, i can't remember why , but for the OCA it's worth a read to check again, remember the why of this decision, and see if v14 changes this, making it possible to fix. |
@thomaspaulb as i implement fixes i see 2.1.0 released, a lot of activity. https://github.com/showdownjs/showdown/releases/tag/2.1.0 but we never use the CLI. so it should be drop-in replacement. hopefully we will have some improvements, Looking at the log , it's mostly documentation and some lib upgrades, but i see work done in the converter too here and there. |
@thomaspaulb update the documentation now directs us to the plugins used by kevin to improve showdown:
|
2.1 is not a release, code will still be tagged as 2.0, but it is not alpha anymore and has all the fixes. |
de8f398
to
12d9b33
Compare
@thomaspaulb done all fixes ( except for the "copied code" one,), updated libs to latest. made a quick test. no regressions |
V14 port of web_widget_html_markdown
rest worked out of the box.