Fixup: convert optgroup child data identifiers to string datatype #6239
Conversation
…f grouped options
… update to initialize using integer keys
… fixup: lint errors
|
Argh, I don't think this is testing the correct behaviour. Currently the code here tests clicks on the 'remove' button on each tag within the multi-select. Re-attempting the demo from #3882, that behaviour works fine. What is buggy is selecting an item from the drop-down, and then attempting to de-select it, but again by using the dropdown. Hopefully that is a small change to the test code.. |
… logic adjustment: attempt to de-select the second item using the dropdown menu
… fixup: variable declaration should be explicit
… filter selected items by aria-selected property instead of highlight CSS class (that doesn't always imply selection)
tests/integration/select2-methods.js
Outdated
| ); | ||
|
|
||
| // Remove the second selection by clicking on the item in the dropdown | ||
| $selections.first().trigger('click'); |
There was a problem hiding this comment.
The idea with this line is to simulate the user clicking on the dropdown menu item, so that we can check that the selection is removed afterwards. It doesn't appear to invoke a relevant event handler though.
There was a problem hiding this comment.
Changing the event from click to mouseup -- to correspond with the event handler here seems to make a difference. It causes the dom-changes.js integration test to break, though:
>> select2(val) - multiple selection and clearing of grouped options
>> Message: Too many calls to the `assert.async` callback
>> Actual: null
>> Expected: undefined
>> http://localhost:9999/tests/integration/dom-changes.js:284:14
… use the qunit fixture as the select2 element's container
… update the mid-test 'choices remaining' jQuery query to be fixture-bounded
… relocate the code that opens the select2 element's dropdown menu
… factor-out the '' jQuery variable
… open the dropdown menu by performing a select2 search
… use the (globally-scoped) dropdown menu results to de-select the last selected item
…select2#6217)" This reverts commit 0a186eb.
|
Ok, the test coverage here for #3882 seems to also validate the fix attempted originally in #6179 (subsequently reverted due to lack of test coverage). Even so: I don't think that the fix from #6179 is ideal. There could be a better approach possible. I'll spend some time to investigate that while looking at this. |
jayaddison
changed the title
|
I'm wondering if it would make sense to apply this fix within |
|
Yep, that sounds sensible - I'll take another look at the code and see how to refactor it like that. |
|
(I guess that's not really refactoring; more like a better fix. anyway, taking a look into that now) |
… to refer to function
…variable to refer to function" This reverts commit 54197a9.
…malizeItem call to a separate line
…provided to a select2 constructor should not contain multiple levels of child nesting; limit normalizeItem recursion to depth one as a result
…he data provided to a select2 constructor should not contain multiple levels of child nesting; limit normalizeItem recursion to depth one as a result" This reverts commit 758a717.
…le-select-with-option-groups
This pull request includes
The following changes were made
string.Resolves #3882.