⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Jan 7, 2026

Currently when the sets manager page loads the "Import how many sets?" select has "a single set" initially selected and the "Import from where?" select has "Select filenames below" selected. But then if you change the first select to "multiple sets" the "Import from where?" select still has "Select filenames below" selected. Furthermore, if you then click on that select and use shift-down arrow to select multiple sets, that first disabled option stays selected. Then form validation fails for that option since it has no value if you click the "Import" button.

This just makes it so that when you switch from the "Import how many sets?" select from "a single set" to "multiple sets", that first option in the "Import from where?" select is immediately unselected.

There is also a little clean up of this section of JavaScript code. The elements with id import_source_select and name action.import.number are actually the same element, so no need to find them in the DOM twice. Also ensure the elements exist and are found before trying to work with them. More of this is needed in this file, but this is probably good enough for now.

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes the issue.

@somiaj
Copy link
Contributor

somiaj commented Feb 1, 2026

This fixes the issue you mention, and currently now if you select multiple sets, no filenames are selected, but if you then select single set again, the first set definition file then selected. Would it be reasonable to return this back to the place holder "Select filenames below". Also "Select filenames below" is plural even in the case only a single file name is selectable. Unsure if that is worth cleaning up (though with translations might need to store both translations as data attributes).

@drgrice1
Copy link
Member Author

drgrice1 commented Feb 1, 2026

We could make the plurality of "filenames" be responsive to the select type, but I don't think that it is a good idea to make it so that the place holder is selected when switching from selecting single to selecting multiple sets. Perhaps you didn't notice that if you have it set to select a single set and select a particular set, and then change to selecting multiple sets, that single set stays selected. So it happens consistently both ways.

Furthermore, there are advantages to that behavior. For example, I have a long list of set definitions, I have selection single selected, and I scroll down and select one set. But then see another set near it that I want to also select, so I want to change to selection multiple. When I do the set I previously select is still selected, and now I can add to that selection.

I see zero benefits from changing that behavior.

@drgrice1 drgrice1 force-pushed the import-default-not-selected branch from 730e9a5 to 5553e7e Compare February 1, 2026 12:04
@somiaj
Copy link
Contributor

somiaj commented Feb 1, 2026

I agree that if something is selected it should stay selected, I was talking about the case that nothing is selected, then switching back to choosing single set causes the first item to become selected.

Looks like in my edits of the message before I posted, I deleted a sentence that mentioned if items are selected then the first item should still stay selected.

@drgrice1
Copy link
Member Author

drgrice1 commented Feb 1, 2026

There doesn't seem to be much that can be done about that case. When the change event occurs the first option after the "select filenames below" option is already selected. Probably this is some default browser behavior that causes it to be selected. And so you can't know if it is selected because the user did so, or because of this default browser behavior.

That is not to big of a deal, and I don't think it will cause major confusion though.

@somiaj
Copy link
Contributor

somiaj commented Feb 1, 2026

Agreed, and really will probably only occur when testing things. Very unlikely someone is going to switch from single set to multiple sets, then back to single set without selecting something. Just an observation I made on behavior in my browser.

Copy link
Contributor

@somiaj somiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well. Fixing the plural issue is very minor, so I approve either way.

…ed issue.

Currently when the sets manager page loads the "Import how many sets?"
select has "a single set" initially selected and the "Import from
where?" select has "Select filenames below" selected.  But then if you
change the first select to "multiple sets" the "Import from where?"
select still has "Select filenames below" selected.  Furthermore, if you
then click on that select and use shift-down arrow to select multiple
sets, that first disabled option stays selected.  Then form validation
fails for that option since it has no value if you click the "Import"
button.

This just makes it so that when you switch from the "Import how many
sets?" select from "a single set" to "multiple sets", that first option
in the "Import from where?" select is immediately unselected.

There is also a little clean up of this section of JavaScript code.  The
elements with id `import_source_select` and name `action.import.number`
are actually the same element, so no need to find them in the DOM twice.
Also ensure the elements exist and are found before trying to work with
them. More of this is needed in this file, but this is probably good
enough for now.
@drgrice1 drgrice1 force-pushed the import-default-not-selected branch from 5553e7e to 63b0827 Compare February 1, 2026 20:34
@drgrice1
Copy link
Member Author

drgrice1 commented Feb 1, 2026

I found out that I was checking the value of the action.import.source select to late in the method. If you check it before the size and multiple attributes are changed, then it does have the selected option when switching from multiple to single. So now if you switch from multiple to single with nothing selected, the first default option is selected.

As to the plurality of the wording, note that this code also has an untranslated string the javascript uses. Namely (taken from filenames) for the action.import.name input. This code is very old, and is from before I started adding this data attribute translation approach. That could be updated.

between selecting single or multiple sets in the import sets form.
This uses translations for the text from data attributes.

The `(taken from filenames)` text that is put into the value for the
"Import sets with names" input is also translated using a data
attribute.
@drgrice1
Copy link
Member Author

drgrice1 commented Feb 1, 2026

I went ahead and fixed the plurality and translated that other string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants