-
-
Notifications
You must be signed in to change notification settings - Fork 168
Fix sets manager import form selection multiple default option selected issue. #2880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Fix sets manager import form selection multiple default option selected issue. #2880
Conversation
pstaabp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixes the issue.
eb7a8d2 to
730e9a5
Compare
|
This fixes the issue you mention, and currently now if you select |
|
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. |
730e9a5 to
5553e7e
Compare
|
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. |
|
There doesn't seem to be much that can be done about that case. When the That is not to big of a deal, and I don't think it will cause major confusion though. |
|
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. |
somiaj
left a comment
There was a problem hiding this 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.
5553e7e to
63b0827
Compare
|
I found out that I was checking the value of the As to the plurality of the wording, note that this code also has an untranslated string the javascript uses. Namely |
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.
|
I went ahead and fixed the plurality and translated that other string. |
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_selectand nameaction.import.numberare 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.