backdrop-contrib / fast_token_browser

An improved user interface for Backdrop CMS's Token browser
https://backdropcms.org/project/fast_token_browser
GNU General Public License v2.0
1 stars 5 forks source link

Refactor JS code to use namespacing #19

Closed argiepiano closed 1 month ago

argiepiano commented 3 months ago

FTB javascript doesn't follow some of the standard Backdrop JS practices - for example it uses global window variables with no name spacing. I'll attempt to refactor the code to make it more in line with how other js files do it in Backdrop

herbdool commented 1 month ago

@argiepiano I've made an attempt at refactoring the JS. I'm not so sure about using the window.Backdrop namespace for part of it but it seems like it works.

argiepiano commented 1 month ago

Ha! I was working on this as well. Let me take a look.

Instead of using window.selectedToken I suggest Backdrop.settings.tokenBrowserSelectedToken

argiepiano commented 1 month ago

I am using a different approach that encapsulates everything (variables etc) within a function, within Backdrop.behaviors.fastTokenBrowser. This way you can still use var, since those variables will not be global. I'm going to post my version of this.

I am not sure about the use of Backdrop. to store all those variables. That will store them as properties of the global object Backdrop, and in my opinion this is not safe and is unnecessary. Those variables (which hold the DOM templates) are only needed within the fast token browser function scope.

argiepiano commented 1 month ago

@herbdool I posted an alternative pr

herbdool commented 1 month ago

@argiepiano let's use yours. I hadn't put as much thought or time into refactoring. Yours is a better approach.