#1098 Fixing file selector appears twice after importing wrong file

[ntran18]

The root cause was that when the user clicks on "Select New File," it triggers ".upload".click(). But the class upload is bound to two Open File buttons: one in the sidebar and one at the top bar, which triggers the file selector twice.

[dondi]

Thanks @ntran18 for explaining the nature of the bug—with that in mind, these comments are more of an exercise to see how minimal the fix can be. Please experiment a bit and let me know

[dondi]

Per the discussion at our meeting, let’s try reverting this change so that we retain having a single line of code to designate this behavior, since it remains the same for both elements

Suggested change

	$("body").on("change", "#upload-network", uploadHandler(loadGrn));
	$("body").on("change", "#upload-nav", uploadHandler(loadGrn));
	$("body").on("change", ".upload", uploadHandler(loadGrn));

[dondi]

Please see if this works:

Suggested change

	input(type='button' id='launchFileOpen' class='btn btn-primary' data-dismiss='modal' value='Select New File' onclick="$('#upload-network').click()")
	input(type='button' id='launchFileOpen' class='btn btn-primary upload' data-dismiss='modal' value='Select New File')

…if this doesn’t work, the general idea is to group this button alongside all of the other controls that trigger the upload handler. That way, we can specify with a single line of code that all of these elements are to invoke the upload handler when clicked/changed

[ntran18]

I tried to give it an upload class, but it doesn't work. However, if we refer to just one of the upload buttons, it will trigger the uploadHandler.

[dondi]

OK, sounds good—yes that will work; the core problem was the $('.upload') selector so as long as we target just one element then we’ll be OK

[dondi]

This is nice! Just one investigative question on the change handling assignment—I can look that up too just in case, but if you already know, just go ahead and respond

[dondi]

OK, sounds good—yes that will work; the core problem was the $('.upload') selector so as long as we target just one element then we’ll be OK

[dondi]

Looks like this just goes back to what it was, but quick question now that I see the commented-out line: are you aware of any difference between the top and bottom approaches? It seems like they would be the same, but clearly someone historically thought that doing $("body").on("change", ..... would be better. I guess we can look up the commit log

[ntran18]

The two approaches are similar, but the .change method is deprecated in jQuery from version 3.3, so Ona reconstructed the code to address this. The first approach attempts to bind the event directly to existing elements with the class upload, meaning these elements should already be present in the HTML file. In contrast, the second approach is better because it uses event delegation by binding the change event to the body element and specifying .upload as the target. This allows for handling events for upload elements that currently exist or will be added to the DOM in the future, meaning we can dynamically add the upload class via JavaScript and still manage the events effectively.

[dondi]

Ah got it—well deprecation is certainly a good reason for changing things, plus yes, the newer version indeed does “late binding” on the selector 👌🏽 Thanks for tracking this down!

[dondi]

Thanks for the explanation on the .upload handler!

[dondi]

Ah got it—well deprecation is certainly a good reason for changing things, plus yes, the newer version indeed does “late binding” on the selector 👌🏽 Thanks for tracking this down!