| Message ID | 20210719233428.2045872-2-raxel@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | patch-list: improve usability of list action bar | expand |
Hi, Raxel Gutierrez wrote: > As per Django docs[1], the library is useful to add csrftoken when > making AJAX requests in JavaScript. More details in the README GitHub > link provided. > > [1] https://docs.djangoproject.com/en/3.2/ref/csrf/#ajax > > Signed-off-by: Raxel Gutierrez <raxel@google.com> > --- The first thing I wonder when looking at the description above is "why wasn't this needed before"? There are no existing users of document.cookie in patchwork. Is the point that all existing code uses {% csrf_token %} in forms generated by the server instead of dynamically generated requests? If so, makes sense. [...] > --- /dev/null > +++ b/htdocs/js/js.cookie-2.2.1.min.js > @@ -0,0 +1,3 @@ > +/*! js-cookie v2.2.1 | MIT */ How do we decide between this going in lib/packages/ versus htdocs/js/? (That's a genuine question --- I don't understand patchwork's current split. Is the idea that lib/packages/ is supposed to contain a package with a README and htdocs/js/ is supposed to contain symlinks to there?) [...] > --- a/templates/base.html > +++ b/templates/base.html > @@ -21,6 +21,7 @@ > <script src="{% static "js/bootstrap.min.js" %}"></script> > <script src="{% static "js/selectize.min.js" %}"></script> > <script src="{% static "js/clipboard.min.js" %}"></script> > + <script src="{% static "js/js.cookie-2.2.1.min.js" %}"></script> Should this use an unversioned URL like the rest of these? Also, how do we decide between putting this in base.html (i.e., all pages) versus specific pages making requests that need a csrf token? The script is small enough that it shouldn't make a difference, but asking anyway because I am curious. Thanks, Jonathan
Hi, > The first thing I wonder when looking at the description above is "why > wasn't this needed before"? > > There are no existing users of document.cookie in patchwork. Is the > point that all existing code uses {% csrf_token %} in forms generated > by the server instead of dynamically generated requests? If so, makes > sense. New patch commit message makes it more clear :) > How do we decide between this going in lib/packages/ versus > htdocs/js/? > > (That's a genuine question --- I don't understand patchwork's current > split. Is the idea that lib/packages/ is supposed to contain a > package with a README and htdocs/js/ is supposed to contain symlinks > to there?) I'm not sure what the htdocs directory is intended for exactly. It seems like static files. But this is a good question to ask given that there seems to be overlap. I'm not sure what exactly differentiates the intended contents of these two directories. As for this patch, patch-list.js follows the current pattern of the bundle.js file being added to htdocs/js. > > @@ -21,6 +21,7 @@ > > <script src="{% static "js/bootstrap.min.js" %}"></script> > > <script src="{% static "js/selectize.min.js" %}"></script> > > <script src="{% static "js/clipboard.min.js" %}"></script> > > + <script src="{% static "js/js.cookie-2.2.1.min.js" %}"></script> > > Should this use an unversioned URL like the rest of these? The version is specified in the htdocs/js README, so perhaps it would be nice to stay consistent and remove it? However, jquery-1.10.1.min.js would continue to break this consistency, so I'm not sure about what's right. > Also, how do we decide between putting this in base.html (i.e., all > pages) versus specific pages making requests that need a csrf token? > The script is small enough that it shouldn't make a difference, but > asking anyway because I am curious. Personally, I think adding those dependencies in the base.html makes it easier later when creating new templates as you don't have to remember which one to add as the number of them continues to grow. Also, I think following the mindset that more content should be handled dynamically client-side with JavaScript means that more templates will be using the library. Actually, I think all of them should eventually be using it.
Raxel Gutierrez wrote: > Jonathan Nieder wrote: >> How do we decide between this going in lib/packages/ versus >> htdocs/js/? [...] > I'm not sure what the htdocs directory is intended for exactly. It > seems like static files. But this is a good question to ask given that > there seems to be overlap. I'm not sure what exactly differentiates > the intended contents of these two directories. Amplifying this question for others on the list more familiar with patchwork's intended source layout. [...] >> Raxel Gutierrez wrote: >>> @@ -21,6 +21,7 @@ >>> <script src="{% static "js/bootstrap.min.js" %}"></script> >>> <script src="{% static "js/selectize.min.js" %}"></script> >>> <script src="{% static "js/clipboard.min.js" %}"></script> >>> + <script src="{% static "js/js.cookie-2.2.1.min.js" %}"></script> >> >> Should this use an unversioned URL like the rest of these? > > The version is specified in the htdocs/js README, so perhaps it would > be nice to stay consistent and remove it? However, > jquery-1.10.1.min.js would continue to break this consistency, so I'm > not sure about what's right. Unversioned feels a little cleaner. (Side note: it looks like version 3 supports ES modules: https://github.com/js-cookie/js-cookie#direct-download. That's probably not enough reason to use a prerelease snapshot of it, though.) >> Also, how do we decide between putting this in base.html (i.e., all >> pages) versus specific pages making requests that need a csrf token? >> The script is small enough that it shouldn't make a difference, but >> asking anyway because I am curious. > > Personally, I think adding those dependencies in the base.html makes > it easier later when creating new templates as you don't have to > remember which one to add as the number of them continues to grow. Fetching the library is not likely to make much difference because the browser will cache the js. But parsing and running it adds a bit to page load time. Probably not enough to be noticeable. > Also, I think following the mindset that more content should be > handled dynamically client-side with JavaScript means that more > templates will be using the library. Actually, I think all of them > should eventually be using it. Yes, that makes sense to me, so looks good. Thanks, Jonathan
diff --git a/htdocs/README.rst b/htdocs/README.rst index 62f15c2..dfa7ca8 100644 --- a/htdocs/README.rst +++ b/htdocs/README.rst @@ -122,6 +122,15 @@ js :GitHub: jQuery plug-in to drag and drop rows in HTML tables :Version: ??? +``js.cookie.min.js`` + + Library used to handle cookies. + + This is used to get the ``csrftoken`` cookie for AJAX POST requests. + + :GitHub: https://github.com/js-cookie/js-cookie/ + :Version: 2.2.1 + ``selectize.min.js`` Selectize is the hybrid of a ``textbox`` and ``<select>`` box. It's jQuery diff --git a/htdocs/js/js.cookie-2.2.1.min.js b/htdocs/js/js.cookie-2.2.1.min.js new file mode 100644 index 0000000..f5f4c36 --- /dev/null +++ b/htdocs/js/js.cookie-2.2.1.min.js @@ -0,0 +1,3 @@ +/*! js-cookie v2.2.1 | MIT */ + +!function(a){var b;if("function"==typeof define&&define.amd&&(define(a),b=!0),"object"==typeof exports&&(module.exports=a(),b=!0),!b){var c=window.Cookies,d=window.Cookies=a();d.noConflict=function(){return window.Cookies=c,d}}}(function(){function a(){for(var a=0,b={};a<arguments.length;a++){var c=arguments[a];for(var d in c)b[d]=c[d]}return b}function b(a){return a.replace(/(%[0-9A-Z]{2})+/g,decodeURIComponent)}function c(d){function e(){}function f(b,c,f){if("undefined"!=typeof document){f=a({path:"/"},e.defaults,f),"number"==typeof f.expires&&(f.expires=new Date(1*new Date+864e5*f.expires)),f.expires=f.expires?f.expires.toUTCString():"";try{var g=JSON.stringify(c);/^[\{\[]/.test(g)&&(c=g)}catch(j){}c=d.write?d.write(c,b):encodeURIComponent(c+"").replace(/%(23|24|26|2B|3A|3C|3E|3D|2F|3F|40|5B|5D|5E|60|7B|7D|7C)/g,decodeURIComponent),b=encodeURIComponent(b+"").replace(/%(23|24|26|2B|5E|60|7C)/g,decodeURIComponent).replace(/[\(\)]/g,escape);var h="";for(var i in f)f[i]&&(h+="; "+i, !0!==f[i]&&(h+="="+f[i].split(";")[0]));return document.cookie=b+"="+c+h}}function g(a,c){if("undefined"!=typeof document){for(var e={},f=document.cookie?document.cookie.split("; "):[],g=0;g<f.length;g++){var h=f[g].split("="),i=h.slice(1).join("=");c||'"'!==i.charAt(0)||(i=i.slice(1,-1));try{var j=b(h[0]);if(i=(d.read||d)(i,j)||b(i),c)try{i=JSON.parse(i)}catch(k){}if(e[j]=i,a===j)break}catch(k){}}return a?e[a]:e}}return e.set=f,e.get=function(a){return g(a,!1)},e.getJSON=function(a){return g(a,!0)},e.remove=function(b,c){f(b,"",a(c,{expires:-1}))},e.defaults={},e.withConverter=c,e}return c(function(){})}); \ No newline at end of file diff --git a/templates/base.html b/templates/base.html index 8accb4c..e1fac82 100644 --- a/templates/base.html +++ b/templates/base.html @@ -21,6 +21,7 @@ <script src="{% static "js/bootstrap.min.js" %}"></script> <script src="{% static "js/selectize.min.js" %}"></script> <script src="{% static "js/clipboard.min.js" %}"></script> + <script src="{% static "js/js.cookie-2.2.1.min.js" %}"></script> <script> $(document).ready(function() { new Clipboard(document.querySelectorAll('button.btn-copy'));
As per Django docs[1], the library is useful to add csrftoken when making AJAX requests in JavaScript. More details in the README GitHub link provided. [1] https://docs.djangoproject.com/en/3.2/ref/csrf/#ajax Signed-off-by: Raxel Gutierrez <raxel@google.com> --- htdocs/README.rst | 9 +++++++++ htdocs/js/js.cookie-2.2.1.min.js | 3 +++ templates/base.html | 1 + 3 files changed, 13 insertions(+) create mode 100644 htdocs/js/js.cookie-2.2.1.min.js