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

cat-select should not use ui-select2 as part of its "settings" interface. #12

Open
mkurz000 opened this issue Mar 9, 2015 · 2 comments

Comments

@mkurz000
Copy link
Contributor

mkurz000 commented Mar 9, 2015

Reason:

  • Currently ui-select2 is used to configure parts of cat-select.
  • This violates the encapsulation, as ui-select2 is IMHO an implementation detail of cat-select.
  • It would be nice to if all relevant settings would be provided as direct cat-select attributes, which could be forwarded to the ui-select2 internally.
  • So the implementation switches to some other control internally later, the interface could stay stable.
@tsalzinger
Copy link
Contributor

at the time this was a design choice for several reasons:

  • the initial version of cat-select was just intended for simple use cases which basically just provide an endpoint and nothing else
  • as several features were added it soon became obvious that just adding new config options one after the other wasn't a viable option, as it it basically was just passing along everything 1:1, therefor a direct access to the select2 config was granted
    • the currentl implementation will never change away from select2 without a mayer api break anyway (planned is the replacement with https://github.com/angular-ui/ui-select, which has no comparable configuration anymore)

if you have any ideas on how to prepare the api already for the switch to the component based ui-select without just adding another api break now, i'd be happy to add it to the project

@mkurz000
Copy link
Contributor Author

We/You should ask ourselves, which features(options) make sense for a component like cat-select, and which do we want to provide?

When digging around in our code, most of the used ui-select2 options are:

  • allowClear: true
  • formatResult: someFormatFunction
  • formatSelection: anotherFormatFunction

Not so frequently used options are (actually each only once):

  • minimumResultsForSearch: -1 // hide the search box.
  • createSearchChoicePosition // usage seems quite obscure to me
  • createSearchChoice // usage seems quite obscure to me

So taking this information, we could come up with the following suggestion:

  • The "conversion" stuff is required, even in a new implementation like ui-select, so "formatResult" and "formatSelection" functionality should be provided to cat-select and be forwarded internally.
  • allowClear should be a direct option as well (although if it is missing in future everything will more or less work without it, I think, its more "UI prettify", as with empty cat-select fields must be dealt with properly anyway)

as for "...was just passing along everything 1:1, therefor a direct access to the select2":

  • IMHO this is a weak argument, as this is what interfaces are for in many cases: forwarding something to internal implementations, to allow changes of the internal implementation without affecting the interface later. And not at least to restrict yourself, meaning: Don't use stuff, which will not work in the future, find another solution for it instead.
  • Specially in our case we only have these 3 frequently used options, add them.
  • For the others (minimumResultsForSearch, createSearchChoicePosition ,createSearchChoice) we should perhaps find another approach anyway.

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

No branches or pull requests

2 participants