Skip to content
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

Cant remove items since v1.4.2 #57

Closed
chosten opened this issue Mar 30, 2021 · 8 comments
Closed

Cant remove items since v1.4.2 #57

chosten opened this issue Mar 30, 2021 · 8 comments
Labels
bug Something isn't working

Comments

@chosten
Copy link

chosten commented Mar 30, 2021

I have some problems when trying to remove items after upgrading to v1.4.2 or v1.4.3.
Unfortunately I was not able to reproduce it in a jsfiddle environment. I believe it is related to the reactive framework I'm using.
So I will try to explain what I noticed and maybe someone will understand what is happening.

This is how the framework I am using is getting the selected values from the select element.

    getSelectValues(el) {
        return Array.from(el.options)
            .filter(option => option.selected)
            .map(option => {
                return option.value || option.text
            })
    },

And that is not working anymore with tom-select v1.4.2 after removing an item because the options from el.options are still selected. And yet el.querySelectorAll('option[selected]') returns the correct selection.

Does that make any sense for someone ?

@chosten chosten added the bug Something isn't working label Mar 30, 2021
@oyejorge
Copy link
Member

I think I know what's happening. Will you try this:

    getSelectValues(el) {
        return Array.from(el.options)
            .filter(option => option.getAttribute('selected') )
            .map(option => {
                return option.value || option.text
            })
    },

@chosten
Copy link
Author

chosten commented Mar 30, 2021

Yes that works.
Does that mean that tom-select should also set the attribute when selecting an option ?

@oyejorge
Copy link
Member

Awesome. Yeah, tom-select should already set the selected attribute when an option is selected. If you're not seeing that, let me know.

tom-select should also support your original code as well as the modification. I'm working on creating a test case that catches the problem you saw but so far not having any luck. Could you tell me how the items in your app were selected in the first place? Were they html elements like ? Or were they dynamically selected in your framework like option.selected = true?

@chosten
Copy link
Author

chosten commented Mar 30, 2021

The select element and the options are included in the html when the page loads but without any "selected" attribute.
They are then selected in JS with option.selected before the initialization of tom-select.
Already tried that in a jsfiddle also but it worked fine. So far I can't pinpoint what's wrong in my case.

On a side note but maybe related, I went back to 1.4.1 and noticed that I was also having problems when adding new items, the JS framework detects only the newly added items but the old ones are detected as unselected, and thus removed. I will try to fix that by setting the selected attribute instead of just option.selected.

@oyejorge
Copy link
Member

Thanks. I've got a test working now and the fix!

@chosten
Copy link
Author

chosten commented Mar 30, 2021

Just tested it and that fixes all of my problems indeed.
Great and fast work, thank you !

@chosten
Copy link
Author

chosten commented Apr 20, 2021

Hey guys, any plan for a release ?

@oyejorge
Copy link
Member

Sorry, yes. Should have a release out in a day or two. Just a few more changes needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants