django-comments-xtd icon indicating copy to clipboard operation
django-comments-xtd copied to clipboard

feat(react): on the fly

Open Mte90 opened this issue 1 year ago • 3 comments

Ref: https://github.com/danirus/django-comments-xtd/issues/418

So this PR implements:

  • Various check to avoid crashes if window.comments_props is not defined or if the comments div not exists
  • Add the support to create div also with a .comments class, not just the ID
  • Load the proprieties also from the HTML itself, in this way you don't need anymore a unique global variable but for every box comments you can have specific with different app initiation
    • Support the old system and the new one
  • Check for new comments with mutationObserver in this way the new comments can be added in the page as you want on the fly

This requires some updates to the documentation:

<div class="comments-wrapper">
  <span class="comments-props" data-comments='{% get_commentbox_props for object %}'></span>

  <div class="comments">
    {% render_xtdcomment_tree for object allow_flagging allow_feedback %}
  </div>
</div>


Mte90 avatar Jan 31 '24 15:01 Mte90

I am working on a fix to avoid a new app execution.

Mte90 avatar Jan 31 '24 15:01 Mte90

This solution, trying to combine the case in which the hook for the ReactJS plugin does not exist with the case in which it does exist is overcomplicated. If you need the JavaScript plugin to be loaded when certain conditions are met, you could inject the plugin then instead.

Covering the two cases makes the code difficult to understand, specially when it is not documented. I appreciate your willing to help extend the functionality of this application. I thank you for that. But frontend-wise, it's much cleaner to fork the project and build the frontend plugin the way you want it apart. The backend code won't change much since it is stable since very long ago.

For an extended explanation on why I don't want to over complicate the frontend code, please read my comment in the PR #420.

Thank you again.

danirus avatar Feb 12 '24 17:02 danirus

Inject the plugin is not an easy task as depends on how the UI is built, instead in this way the library itself monitor for new stuff in the DOM and supports also this way to load the options.

I can document it better if this is the problem. I understand your issue to not overcomplicate the frontende code but looking at the tickets and the project history (to develop a new UI) those changes are tiny workaround that let's to get everything without to wait for a new one.

Mte90 avatar Feb 12 '24 17:02 Mte90