indrimuska / angular-selector

A native AngularJS directive that transform a simple <select> box into a full html select with typeahead.
http://indrimuska.github.io/angular-selector
MIT License
96 stars 36 forks source link

Add option to allow user inputted tags #11

Closed kmturley closed 8 years ago

kmturley commented 8 years ago

Option which allows the user to type in their own tags and hit ‘Enter’ to add them to the list available. Add user=“true” to enable this feature

<select selector multi="true" user="true" class="jumbotron-selector">

Original feature request ticket: https://github.com/indrimuska/angular-selector/issues/10

indrimuska commented 8 years ago

Ok @kmturley, this is a good point of start! I've got some little changes to ask you before merging it.

What are your thought?

kmturley commented 8 years ago

Ok custom tags looks good now! should work with custom label, value attributes and also if you reload the dropdown from a model it will detect the custom tags and allow them to be shown

indrimuska commented 8 years ago

Hi @kmturley, very good job! You're almost there! I will comment line by line your code with the updates I would do. Thank you for your interest in making this directive greater! :)

kmturley commented 8 years ago

I think the create function idea is more flexible, so you can convert the tags added however you'd like. But it's a pain also as you can't just set an attribute to enable custom tags.

Also the way you have discussed with a function passed, would need to format not just the custom tags but all tags added?

I prefer the idea that you can add create="true" and user can add their own tags without needing any other configuration!

indrimuska commented 8 years ago

Hi @kmturley, I understand your opinion but I cannot force people using the basic configuration with valueAttr and labelAttr properties. I could tell you so many scenarios where this approach does not help users, i.e. if you use array of strings you can't use this feature, but you can if you use a function like this:

$scope.myCreateFn = function (input) { return input; };

However, I know that having a simple attribute would make this feature ready-to-use. So what about using these approaches together? I mean, we could check if create property is a valid function and apply it to the input, or add the option with the basic configuration only if create attribute exists:

case KEYS.enter:
    if (scope.isOpen) {
        if (scope.filteredOptions.length)
            scope.set();
        else {
          if (attrs.create) {
            var option = {};
            if (angular.isFunction(scope.create)) {
              option = scope.create({ input: e.target.value });
            } else {
              option[scope.labelAttr] = e.target.value;
              option[scope.valueAttr] = e.target.value;
            }
            scope.options.push(option);
            scope.set(option);
          }
        }
        e.preventDefault();
    }
    break;

In this case, you could use both approaches appending the attribute create="true" or create="ctrl.myCreateFn(input)" to your element.

What are your thoughts?

kmturley commented 8 years ago

Great idea, i've adjusted so the developer can pass through either a function or have the default functionality. definitely a better approach!

Additionally I noticed many tag inputs allow the user to enter tags by hitting comma ',' key as well as Enter. So have added this. When the user enters custom characters into Tags it can cause issues, so retain those for labels, but values they are converted into a web safe value using slugify

indrimuska commented 8 years ago

Ok for pushing an option into the array after hitting comma, but be careful with it: you can use it in the same way you use Enter btn only when tag creation is enabled, but what about hitting comma when you scroll down the list with the keyboard arrows? (if you use it like Enter, you know what will happen..)

Also, users can have options with one or more commas inside their strings, so it would be nice if this feature might be enabled/disabled by users.

My last consideration is about your slugify function: in my opinion this function could be uncomfortable for those who needs raw inputs, and also I think that it is perfect for use with a custom create function. We have to change data as little as possible and give the user the maximum flexibility in order to help them achieve their goals.

EDIT: I created this plunker with the changes I like so far, it lacks only one thing for me to publish this feature: a little option in the dropdown informing the user that the option will be created.

kmturley commented 8 years ago

Yeah I agree the slugify function could be achieved with the create function. If you look at how selectivize works it has the comma functionality: http://selectize.github.io/selectize.js/

To have an item in the dropdown saying 'Add mytag' you would need to reserve the last place always for whatever the current user is typing, so the tag is pushed in on creation, and a reference is kept throughout.

scope.newTag = {};
scope.newTag[scope.labelAttr] = 'Add';
scope.newTag[scope.valueAttr || 'value'] = '';
scope.options.push(scope.newTag);

Then as user types update the label and value:

scope.newTag[scope.labelAttr] = 'Add ' + e.target.value;
scope.newTag[scope.valueAttr || 'value'] = e.target.value;

And when they hit Enter or Comma reset the label and set it as selected:

scope.newTag[scope.labelAttr] = scope.newTag[scope.valueAttr || 'value'];
scope.set(option);
indrimuska commented 8 years ago

Yes @kmturley, I know that selectize has the comma functionality, but your code now doesn't work well with it: try yourself this plunker, choose an option and then hit "comma", you will see the option will be selected. Anyway, that's not a problem, we could just publish tag creation feature by now, and then support the comma functionality later.

However, I think that we don't really need to have a real option into the array with the "Add tag" label, we could achieve that simply by updating the template with an option like this:

<li ng-if="create && filteredOptions.length == 0" class="selector-option active">
   Add <i ng-bind="search"></i>
</li>

What do you think about it? http://plnkr.co/edit/IEAoE8J41Lf5DCTbiNjx?p=preview

kmturley commented 8 years ago

I agree comma functionality is a little strange if it's using item from list. Could be a separate feature. I like your implementation of dropdown suggestion, have added it. I think could be good to release?

indrimuska commented 8 years ago

Yes, looks good to me, let's go for it! It just needs some little changes:

-       input.parent().append(shadow);
+       angular.element(document.body).append(shadow);
-                           var inList = $filter('filter')(scope.options, function (option) {
+                           return $filter('filter')(scope.options, function (option) {
                                return scope.optionEquals(option, value);
                            })[0];
-                           return scope.createCustom ? value : inList;

After that, I can merge the PR. I'm already working on an example for the demo page to show how this feature works. Thank you man! :)

kmturley commented 8 years ago

Check this version. Where the custom tags are loaded in as the model: http://plnkr.co/edit/HChqis9aT8yydMEnf9kF?p=preview

Notice how customtag is allowed in the model onload. Now remove the inList line and notice that custom tag is ignored onload

indrimuska commented 8 years ago

Ok, I analyzed your plunker but I really can't understand why you want to choose options that are not available in the select. Why you can't just push them into your options? Also, your pre-selected options are no more available for selection after I remove them, and I think it will cause more problems than solutions. Remember that you can access the options array after initialization, even if you fill the select via HTML (check out "Fill options from HTML" example).

<select selector multi="true" create="true" model="preselected" options="myOptions">
      <option value="item1">Item 1</option>
      <option value="item2">Item 2</option>
      <option value="item3">Item 3</option>
</select>
<pre ng-bind="myOptions | json"></pre> <!-- array is filled from HTML <option>s -->

You can also use the selected attribute of the option tag:

<select selector multi="true" create="true" model="preselected">
      <option value="item1">Item 1</option>
      <option value="item2" selected>Item 2</option>
      <option value="item3">Item 3</option>
      <option value="customtag" selected>customtag</option>
</select>

Anyway, I found a little issue with the style: when all the options are selected and the input text has no values I see the "Add " label. So, to prevent this, template must be like this:

<ul class="selector-dropdown" ng-show="filteredOptions.length > 0 || (create && search)">
    <li class="selector-option active" ng-if="filteredOptions.length == 0">
        Add <i ng-bind="search"></i>
    </li>
...
kmturley commented 8 years ago

In your example you are presuming the server renders the html, which means you can output the selected attribute:

<option value="item2" selected>Item 2</option>

In my version i'm using them differently:

Example: User A goes onto the form, select from a list of favourite hobbies:

options = [Fishing, Soccer, Music]
model = []

They select Soccer and then a second custom tag: Skydiving

options = [Fishing, Soccer, Music]
model = [Soccer, Skydiving]

The model is saved to the server. User B visits the site and should not see Skydiving, just the original options:

options = [Fishing, Soccer, Music]
model = [Fishing]

If User A returns to the site, I load the ajax feed and populate the model:

options = [Fishing, Soccer, Music]
model = [Soccer, Skydiving]

Skydiving tag is NOT shown in your example, because the tag in the model does not match the item in the options. in my version the tag is allowed to be visible using:

var inList = $filter('filter')(scope.options, function (option) {
    return scope.optionEquals(option, value);
})[0];
return scope.create ? value : inList;

Additionally. I found a bug if you use:

create:                '@?',

and then:

create="true"

then this next statement does not work, it passes through true:

angular.isFunction(scope.create)

But if you use:

create:                '=?',

It works. If you test the plugin using label/value objects and create="true", you should be able to see the issue! I put that example in my previous plunkr: http://plnkr.co/edit/HChqis9aT8yydMEnf9kF?p=preview

indrimuska commented 8 years ago

Hi @kmturley, I know that my solution won't let you see the "custom" inputs, but this is how it should be: simply, since with a standard HTML select you can't select an option that doesn't exists in your list, the same should be for Angular Selector. So if you want to add any option that not exists in your array, you must push it first or use the "create" feature. That's all.

Concerning the bug, you're right: the binding shouldn't be "@?", it should be "&?" (optional function binding). Thank you.

kmturley commented 8 years ago

Ok I understand the need to keep it similar to html. I will remove this, but would be a nice feature :)

indrimuska commented 8 years ago

Well done, thank you man for making this feature real! :)

kmturley commented 8 years ago

No worries. Thanks for making a great plugin!