Skip to content
This repository has been archived by the owner on Oct 2, 2019. It is now read-only.

Tagging support (continues #260) #327

Merged
merged 43 commits into from
Nov 25, 2014
Merged

Tagging support (continues #260) #327

merged 43 commits into from
Nov 25, 2014

Conversation

brianfeister
Copy link

@dimirc I've rebased the latest from @tvervest and @juanpasolano from #260. I've also added auto-tokenization support at your request, and custom labeling (with optional no-label) for new tags. Note that javascript ignores trailing / leading whitespace characters inside data attributes so users will need to use the constant / literal SPACE if they want tagging with that character. You can pass in pipes (so the only invalid character for tagging is |) to get multiple tag characters. This can be seen in the tagging demo I've created in my fork here.

Usage

  1. tagging - this attribute accepts an optional string which is a scope function. That function will be passed a string from the select input and return the transformed result will be injected into ctrl.items (available for selection). e.g. <ui-select multiple tagging="tagTransform">
  2. taggingTokens - accepts a string that will be listened for in keyup, this keyboard character auto-tokenizes the current value and selects it. Multiple tokens are supported by separating characters with |'s. SPACE is a special case since whitespace characters are ignored in input fields by jQuery, as a result, tagging-tokens=" " fails to eval to whitespace, so the special key value SPACE is used instead. e.g <ui-select multiple tagging tagging-tokens="SPACE|,|/"> would listen for space, comma, and forward slash.
  3. taggingLabel - (false|string) accepts an optional string for the label appended to tags in the dropdown (defaults to (new) if empty or undeclared. If the string false is declared it will omit the injected helper ({string} (new)) value from the dropdown and tagging will trigger via tokenization shortcuts or ENTER key.

juanpasolano and others added 19 commits September 18, 2014 11:28
To prevent the hash to reload the page
Conflicts:
	dist/select.css
	dist/select.css.min.css
	dist/select.js
	dist/select.js.min.js
Conflicts:
	dist/select.css
	dist/select.css.min.css
	dist/select.js
	dist/select.js.min.js
Conflicts:
	dist/select.css
	dist/select.css.min.css
	dist/select.js
	dist/select.min.js
	src/select.js
Conflicts:
	dist/select.css
	dist/select.css.min.css
	dist/select.js
	dist/select.min.js
	src/select.js
Conflicts:
	dist/select.js
	src/select.js
@brianfeister
Copy link
Author

@dimirc is there a problem with the Gulp build? When I make changes to /src/select.js with gulp watch in the background, it rebuilds /dist/select.js and injects the templates but omits the contents of /src/select.js. Any ideas?

@brianfeister
Copy link
Author

Apologies, I'm good now - it was failing jshint and I didn't notice. All good now, think the next build should pass.

@brianfeister brianfeister mentioned this pull request Oct 21, 2014
…ng with labels in drop down and tagging without labels
@brianfeister
Copy link
Author

@dimirc, would love some review on this. The addition of a (new) tag appearing in the dropdown was far from trivial but I have it (as well as the option to omit) in there.

Usage

  1. tagging - this attribute accepts an optional string which is a scope function. That function will be passed a string from the select input and return the transformed result will be injected into ctrl.items (available for selection). e.g. <ui-select multiple tagging="tagTransform">
  2. taggingTokens - accepts a string that will be listened for in keyup, this keyboard character auto-tokenizes the current value and selects it. Multiple tokens are supported by separating characters with |'s. SPACE is a special case since whitespace characters are ignored in input fields by jQuery, as a result, tagging-tokens=" " fails to eval to whitespace, so the special key value SPACE is used instead. e.g <ui-select multiple tagging tagging-tokens="SPACE|,|/"> would listen for space, comma, and forward slash.
  3. taggingLabel - (false|string) accepts an optional string for the label appended to tags in the dropdown (defaults to (new) if empty or undeclared. If the string false is declared it will omit the injected helper ({string} (new)) value from the dropdown and tagging will trigger via tokenization shortcuts or ENTER key.

@brianfeister
Copy link
Author

@pderksen
Copy link
Contributor

This is awesome! Been waiting for this for a while (see #226).

Thanks for all your hard work getting this done so we can finally migrate from ui-select2.

@amcdnl
Copy link
Contributor

amcdnl commented Nov 25, 2014

Can you give me a demo and a short doc on how to use this?

@brianfeister
Copy link
Author

@brianfeister
Copy link
Author

@ProLoser was it correct to remove the /dist files? I'm thinking they're necessary, but @dimirc had requested. Now that it's merged I'm pretty sure that's going to break bower installs, no?

@ProLoser
Copy link
Member

@brianfeister I'm not really am active maintainer so I'm extending an invite to you.

I think @dimirc wants to follow the convention of generating the dist files in their own respective commit that would then be tagged vs regenerating them with every patch. You probably should not have touched the dist files in any way.

I have no opinion on best practice but perhaps you can regenerate them on a new tagged commit

@brianfeister
Copy link
Author

Great thanks for the access, appreciate the vote of confidence. I pushed out a release tag and an associated commit: https://github.com/angular-ui/ui-select/releases/tag/v0.8.4

@amcdnl
Copy link
Contributor

amcdnl commented Nov 25, 2014

@ProLoser @brianfeister ya he requested me to remove dist files so those can be generated on release to avoid merge collisions.

@ProLoser
Copy link
Member

@amcdnl @brianfeister I don't think you're supposed to delete and add dist files all the time. Just leave them untouched in patches and only modify them in their own dedicated release commits.

@brianfeister
Copy link
Author

@ProLoser, yeah that was already my understanding. It's just that I misunderstood @dimirc's request and then it got merged with the files removed rather than just having their changes uncommitted.

@dimirc
Copy link
Contributor

dimirc commented Nov 26, 2014

@brianfeister to keep it consistent, I created release v0.9.0

@brianfeister
Copy link
Author

@maxcook - what would make you think that this comment

How could you not see that coming?

would further the cause of making the world a better place? The answer to your question is that you're asking the wrong question. I didn't write the line of code that you're referencing (also you did not correctly use Github markdown and you didn't give a line number reference, so you should probably do that). The code came from an upstream merge of another developers feature and it merged fluidly via Git so I never had the chance to look at that particular state of the directive. So, let's work together and fix it now. Please post a Plnkr.co / jsfiddle.net demo that reproduces the error and we'll get it fixed. Also you should probably open this as an issue rather than piggybacking on a closed merge request.

Your other questions:

  1. "Also it would be awesome to empty the input field after adding a tag (so i do not need to delete the contents myself before adding the next tag)." I've already written the code to empty the input field after adding a tag, hopefully your problem is related to the above issue, that arose from a bad merge.
  2. "DEL key does not delete the contents of that input field". Again, the delete key was working fine in my tests, please post a plnkr / fiddle / etc. (in a new bug report, feel free to @ mention me) that demonstrates the problem.

@santialbo
Copy link

I would like to thank @brianfeister for the great work on this PR. It works like a charm!

@ceesvanegmond
Copy link

I've applied the newest version of ui-select and enabled tagging.
It works, however I do see another problem/error.

When really quickly adding items by typing enter, typing enter, typing enter (really quickly), I do get an error. Any idea how to solve this? Do other people have the same problem?
schermafbeelding 2014-11-26 om 15 46 31

@brianchance
Copy link

@brianfeister - big thanks for your work, really needed this.

@maxcook - I think you can auto clear the text using resetSearchInput attribute on the ui-select element(reset-search-input="true"). You can also set it globally using config:

app.run(['uiSelectConfig', function(uiSelectConfig) {
        uiSelectConfig.resetSearchInput = true;
    }
]);

@brianfeister
Copy link
Author

Thanks for the support everyone, appreciate that.

@ceesvanegmond - can you try this for me and see if it fixes the issue:

https://gist.githubusercontent.com/brianfeister/0c9bffea7ca0b2988dcb/raw/333b70393bb74a500db410da7d43de1e68621607/select.js

If not, please give me a demo of the bug.

@parliament718
Copy link

@ceesvanegmond @brianfeister
I was getting the 3rd error I'm seeing in your screenshot (uiSelectChoiceDisabled)

This is due to line 626: ctrl.select(null, true); passing null at first paramters and then later checking if (item === undefined || !item._uiSelectChoiceDisabled) { ... }

Cannot read _uiSelectChoiceDisabled of null makes sense.
The easiest fix for me was to pass undefined instead:

ctrl.select(undefined, true);

note: fixing this also got reset-search-input working correctly again, which was not resetting due to the exception.

@darrudi
Copy link

darrudi commented Dec 8, 2014

Feature Request:
The user may enter a string which is considered invalid by the tagging function. So the tagging function should be able to return null upon receiving the input. I did this but a bunch of errors surfaced. My tagging function:

    $scope.uiSelectNewTag = function(email){
        email = email.trim();

        if (email.match(/^[a-z][a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,4}$/i)) {
            var newItem = {
                type: 2,
                email: email,
                key: email
            };

            return newItem;

        } else {

            return null;

        }
    };

Also the user may copy-paste a string (e.g. a list of emails separated by semicolons) into the ui-select and the tagging function may decide there are more than one item in the input. So I guess its a good idea to have the tagging function return an array (zero or more items) instead of a string.

@brianfeister
Copy link
Author

@darrudi - please open a separate issue for this so that your request doesn't get lost.

@darrudi
Copy link

darrudi commented Dec 8, 2014

@brianfeister - done: #485

@windmaomao
Copy link

thank you @brianfeister. I just run the example, exactly what i wanted, so now i just need to learn how to use it. Thanks all for the hard work.

@pruchai
Copy link

pruchai commented Jan 14, 2015

Is there any way to disable new tag creation? I want users to only be able to pick tags available in the drop down menu.

EDIT: Never mind. I just disabled tagging and left multiple

@brianfeister
Copy link
Author

;)

@darrudi
Copy link

darrudi commented Jan 15, 2015

@pruchai: Take a glance at examples. All this thread is/was about adding free tag support. :)

@trickpattyFH20
Copy link
Contributor

Great feature to add tagging but I was really hoping this would work for just a regular select dropdown not just the "multiple" select. I have a single select and I'd like it if a user could add options to this list. I tried using tagging on a single select in the demo code and in my own code and it doesn't seem to work.

Do you have plans to implement this for the regular select dropdowns? @brianfeister

or does anybody else have a workaround to implement tagging for single select dropdowns?

@pwchan
Copy link

pwchan commented Jan 16, 2015

+1

@GFoley83
Copy link

GFoley83 commented Feb 8, 2015

@ceesvanegmond @brianfeister @parliament718
As of version: 0.9.6, with tagging enabled, I'm still getting
Cannot read _uiSelectChoiceDisabled of null.

Updating the following line:
https://github.com/angular-ui/ui-select/blob/master/dist/select.js#L675 to
ctrl.select( newItem || undefined, true);
solved it for me. In other words, passing undefined instead of null, as @parliament718 mentioned.

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

Successfully merging this pull request may close these issues.