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

Select2 4.0 support wip #30

Closed
wants to merge 9 commits into from
Closed

Conversation

motin
Copy link

@motin motin commented Sep 19, 2015

JS Bin here: https://jsbin.com/gipezidemi/edit?html,js,output

To make it work with select2 4.0 showed out to be a minor task, thanks to the fact that the underlying element is now a select-element and not an input-element. Most of the directive's code could be removed, and the select2 examples from the readme works as expected.

However, I have not updated any of the tests in the repo (nor will I), and there are probably additional features that were included in the previous angular-select2 that are now broken, so additional work may need to be done. Anyway, it is a start.

#18

@motin motin force-pushed the select2-4.0-support-wip branch from 6194ea8 to 0d7e647 Compare September 20, 2015 00:34
@rubenv
Copy link
Owner

rubenv commented Sep 20, 2015

However, I have not updated any of the tests in the repo (nor will I)

This is obviously a big blocker for any release. The whole reason we have tests is to make sure we don't break things. We'll need to look at what tests and features are still relevant but simply tossing everything out isn't all that acceptable.

@motin
Copy link
Author

motin commented Sep 20, 2015

The development of v4-support is by no means a one-man-show, so while one person does some initial implementation and provides proof-of-concept via jsbin or similar, another one can look at what tests and features are still relevant and yet another can supply updated tests.

controller.$render();

});
});

element.on("select2-blur", function () {

Choose a reason for hiding this comment

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

I might as well mention that there is no longer a select2-blur event that is triggered.

@motin
Copy link
Author

motin commented Oct 21, 2015

I've pushed a v4 branch, pull requests are welcome. Once this branch is stable, we'll merge it to master and we'll split off a maintenance branch for the older select2 variant.

According to the above quoted plan, should not this pr be merged so that others can try it out and submit more pull requests to this branch, for instance with test updates, clean-ups and other fixes?

@rubenv
Copy link
Owner

rubenv commented Oct 22, 2015

History learns me that if a patch submitter doesn't submit tests, he'll never do it. Nor will anyone else.

I'm all for having a v4 branch, but starting out with no tests at all feels like a very bad idea.

@motin
Copy link
Author

motin commented Oct 22, 2015

The test I supplied was in the form of a JS Bin here: https://jsbin.com/gipezidemi/edit?html,js,output

Can we maybe use the same or similar contents/markup/scripts to create tests that are committed in the repo as well?

@rubenv
Copy link
Owner

rubenv commented Oct 22, 2015

Shouldn't be too hard to convert the existing Protractor tests to something that works with v4.

@axar
Copy link

axar commented Mar 22, 2016

what about initial selection with ajax calls?

@tonidy
Copy link

tonidy commented May 2, 2016

@motin any update on this PR?

cc: @rubenv

@motin
Copy link
Author

motin commented May 16, 2016

@tonidy Unfortunately not. There are still missing tests, which are mandatory in order to merge the PR. I still suggest that someone else write those tests since I have all but stopped using angular-select2 in my own projects. It would be great to have an answer on the comment from 22 oct as well: #30 (comment)

@rubenv rubenv closed this Sep 4, 2019
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.

5 participants