Your metaphorical engine weighs a metric ton and it’s very difficult to imagine how it’ll develop into anything resembling a racer. I’d put more effort into the car analogies but they’re making me physically ill.
> The architecture is state-of-the-art.
You should also probably back this statement up with something. Right now it appears to be a lot of self-horn-tooting.
But hey, sure, let’s spend the same half-minute to talk about the code. I open to a random TypeScript file – thank goodness you’re using TypeScript, because the JavaScript you’ve put under version control for some reason would make any non-vendor code in the language difficult to find in this time – https://github.com/Clay-Ferguson/meta64/blob/d9f3740118e8099....
declare var Dropzone;
Oh okay I guess we’re not using types here
let config: Object = {
⋮
var submitButton = document.querySelector("#" + thiz.id("uploadButton"));
This… this isn’t how you reference elements in your “state-of-the-art” architecture, right?
if (!submitButton) {
console.log("Unable to get upload button.");
}
submitButton.addEventListener("click", function(e) {
I guess the state of the art means you can’t use your browser’s debugger. Or it’s just for consistency given that you’re circumventing the rest of the browser too.
this.on("queuecomplete", function(file) {
meta64.refresh();
});
What? I have no idea what this does (`meta64` isn’t declared, I guess because this file is allergic to modules. it also appears to be a singleton or global, neither of which particularly screams “good design” when named in reference to your app) but it looks like some kind of indiscriminate state update given that nothing is being passed to it. Maybe it’s not that, but no time to find out. We’re running out of seconds here.
Wait, back up a moment:
submitButton.addEventListener("click", function(e) {
//e.preventDefault();
dropzone.processQueue();
});
Isn’t a major aspect of most of these fancy SPA frameworks that you don’t have to do this?
$("#" + this.id("dropzone-form-id")).dropzone(config);
> This… this isn’t how you reference elements in your “state-of-the-art” architecture, right?
oh no it is. Bonus points for kebab-case when the other two we’ve seen so far are camel. I’m nitpicking style because a meaningless literary device involving cars made me irritable. Sorry.
let ret: boolean = false;
for (let file of this.fileList) {
if (file["name"].toLowerCase().endsWith(".zip")) {
return true;
}
}
return ret;
That was a useful variable! I would write this:
return this.fileList.some(isZipFile);
and then probably not write this in the end because it’s a filename check but we’re here to talk about architecture or something, not that:
const isZipFile = /\.zip$/i!test;
`!` is a macro for bound property access in my state-of-the-art architecture.
runButtonEnablement = (dropzoneEvt: any): void => {
if (dropzoneEvt.getAddedFiles().length > 0 ||
dropzoneEvt.getQueuedFiles().length > 0) {
$("#" + this.id("uploadButton")).show();
}
else {
$("#" + this.id("uploadButton")).hide();
}
}
I would think a F1 racer ninja rockstar architecture would be able to bind the visibility of this button to some state. More bonus points for not using `toggle()` and the same select-by-id antipattern.
$("#" + this.id("uploadPathDisplay")).html("Path: " + render.formatPath(attachment.uploadNode));
Select-by-id again. I’ve suppressed the urge to gag by this point. `.html()` instead of binding this to some appropriate component? Nothing new. Mysterious global `render` due to declaring things allergy? Par for the course.
Let me know if you would like a review of any other files so I can decline and spare my mental health thanks