django-bootstrap-modal-forms icon indicating copy to clipboard operation
django-bootstrap-modal-forms copied to clipboard

Added documetation & credential property

Open aDramaQueen opened this issue 2 years ago • 4 comments
trafficstars

Hi, since I had to work through the JavaScript for Issiue #213, I have this weird behavior writing documentation for undocumented code. So, here we are. This is basically what it is, to this pull request.

But there is one tiny bit more... I added a new property to the default setting. The credentials property. This is a important security property (see: here), you may give the user the controll over this paramater. I gave it the default value: same-origin. This means, the code didn't change at all. But the user is now able to controll this.

Thats it. Thanks.

aDramaQueen avatar Apr 09 '23 15:04 aDramaQueen

@aDramaQueen This is a great idea and I would like to discuss potential to extend it even further.

  1. What do you think about allowing user to control all the options for the fetch() method? https://developer.mozilla.org/en-US/docs/Web/API/fetch
  2. And what about Bootstrap 4 version? There is also a potential to allow user to completely control ajax calls https://github.com/trco/django-bootstrap-modal-forms/blob/master/bootstrap_modal_forms/static/js/jquery.bootstrap.modal.forms.js#L35.

trco avatar Jul 01 '23 10:07 trco

To 1.) Yes, absolutely. That would only be consequential. To 2.) There was a reason, why I only touched the Bootstrap 5 Code. I don't now jQuery very well.

A design decision must then be made here. Should the user set these options in:

  • JavaScript
  • Python

First one would be easier to implement. You basically write & pass the JavaScript object. Second one would be more conveniant since you would stay in Python, but it would require some kind of exchange. You need to inject this information. I would suggest a custom Django - Tamplate Tag:

<div class="modal fade" tabindex="-1" role="dialog" id="modal">
  {% my-custom-tag %}
  <div class="modal-dialog" role="document">
    <div class="modal-content"></div>
  </div>
</div>

With this, you could inject another div, that would hold all necessary data.

aDramaQueen avatar Jul 03 '23 09:07 aDramaQueen

@aDramaQueen I believe JavaScript option would be more reasonable, since user would be able to set everything when initialising modal form with modalForm().

Any chance that you update this PR and prepare Boostrap5 implementation? I would then add it for Bootstrap4 afterwards. As always your contribution would be highly appreciated.

Thanks for great improvement idea.

trco avatar Sep 02 '23 09:09 trco

Sorry for the delay, there was a lot to do at work.

I re-based the branch and added the changes. This should make merging easier.

Now all Fetch API parameters are set to the recommended default values. I also added the changes to the README.rst.

aDramaQueen avatar Nov 30 '23 17:11 aDramaQueen