Various updates to get it working#92
Various updates to get it working#92theaustinhowe wants to merge 40 commits intopsychobunny:masterfrom
Conversation
|
Hm... are you using the latest version of... templates.js? or https://github.com/benchpressjs/benchpressjs ? We'd really appreciate using the latter, since it is a rewrite of the former for speed and API conformability. It should be drop-in replaceable. |
|
I really appreciate the PR, thank you for contributing back 😄 |
I went to the last version of template.js (before it converted to benchpressjs). I was considering moving to benchpressjs but wasn't sure about size issues of the compiled file - since it didn't appear that I could easily take out the parsing. But if you like, I can look at a client distro version and replace template.js and see what happens? LMK your thoughts - I figured the prior implementation was stripping template as much as possible to keep it light. (Even in this pull from template.js I removed references to things like |
|
@julianlam what exactly is the expectations with benchpressjs?
Maybe there is a more client friendly version I'm missing? If benchpressjs has a version where we can |
Get it to work again...
Support turning profile links off
|
Wow, so much work on this plugin 😄 We can stick with templates.js if you're not comfortable figuring it out, it was just a passing thought. If it works fine right now, there's no need to change it. |
|
I've pinged @pitaj for his thoughts on the tjs ➡️ bch migration. |
| const Comments = module.exports; | ||
|
|
||
| // CORS Middleware | ||
| const CORSMiddleware = function (req, res, next) { |
There was a problem hiding this comment.
Please add a comment explaining what the purpose of this is.
| res.header('Access-Control-Allow-Origin', url); | ||
| } else { | ||
| winston.warn(`[nodebb-plugin-blog-comments] Origin (${req.get('origin')}) does not match hostUrls: ${hostUrls.join(', ')}`); | ||
| let postData = []; let categoryData = null; let |
| nodebbDiv = document.getElementById('nodebb'); | ||
| const nodebbDiv = document.getElementById('nodebb'); | ||
|
|
||
| function newXHR() { |
There was a problem hiding this comment.
Can probably switch to fetch here, but for now you can remove the extremely old ActiveX stuff and use XMLHttpRequest directly.
| } | ||
| }, 500); | ||
| } | ||
| const XHR = newXHR(); let pagination = 0; |
There was a problem hiding this comment.
Put these on separate lines. We should add https://github.com/nodebb/eslint-config-nodebb to align with other plugins.
|
As for tjs to benchpress, it should be possible. An alternative to registering a loader is directly setting the template in Benchpress.cache['comments'] = Promise.resolve(templateFunction);
Benchpress.render('comments', data, block).then(...);I think the best approach would be to bundle |
|
I just tried setting up this plugin when I came across this PR and I can't believe these changes have not been merged yet. All these changes are either required or simply a great addition like the autocreate option. To quote @theaustinhowe:
Btw the General - PHP example in the README seems outdated as well. |
|
@waschinski I pinged @pitaj for his feedback and I think we were waiting on @theaustinhowe to provide changes before merging 😄 |
|
Thanks for the update, meanwhile I've been deciding not to use nodebb due to this and some other issues. |
4 participants
When attempting to utilize the plugin ran into a few issues.
When initially loading a blog; because the topic didn't exist - the response would not allow for displaying the template to admin/publishers.
This was resolved by moving the topic-related content into a different promise group; and allowing the request to go through even if null.
Template shows "Powered By" and original thread link; which we didn't want/need.
This was resolved with a new setting to turn off that functionality
blog-comments:show-brandingTemplate was having issues with properly rendering (I'm not sure how anyone is using this now).
This was resolved by pulling the latest version of the template.js parser and replacing it in
general.js(this may require updating in other public JS libraries)Ability to change login / registration URLs
This was resolved with 2 new settings to update the URL
blog-comments:login-urlandblog-comments:register-urlAbility to redirect instead of modal for authentication purposes.
This was resolved with a new setting:
blog-comments:auth-behaviorAbility to automatically create a topic.
This was resolved with 2 new settings one to turn on the functionality
blog-comments:autocreateand one for default userblog-comments:autocreate-user-idAbility to turn off profile links
This was resolved with a new setting:
blog-comments:profile-links