Skip to content

Allow creation of tokens on the fly #219

Open
wants to merge 7 commits into from
@manuelbernhardt

No description provided.

@masterkain

+1

@adammck adammck added a commit to adammck/djtokeninput that referenced this pull request Dec 6, 2011
@adammck adammck Spawn instances of the model for unrecognized keys.
This allows models (for example, tags) to be spawned and related within
the form itself. This requires JS support on the client-side, which
loopj/jquery-tokeninput@1273e68
doesn't include. Check out loopj/jquery-tokeninput#219 for a patch.

If and when the patch is merged into master, I'll merge this branch.
e9e0364
@manuelbernhardt manuelbernhardt Fixing issues with token creation
- created tokens are not cached, so they do not lead to wrong lookups from the cache
- fixing displaying of the new label hint text
a899d1b
@groenroos

+1 This truly is the feature missing from tokenInput.

I had some UX epiphanies while adding this patch to my project though. In my particular application I at least prefer the custom token to be at the top instead of the bottom, so I added an option for that. And, in case there already is exists a proper token with the exact same name as the user input, no need to display a custom token. I put them in a gist, if you're interested; https://gist.github.com/1492593

@Bringo
Bringo commented Dec 18, 2011

+1

@manuelbernhardt

@groenroos it'd be nice to have your improvements in there - but it's a little hard to get a diff via a gist. Could you perhaps fork my fork (now that sounds weird) and add your changes on top? Then I could pull them in (and eventually @loopj can then pull the whole thing in)

@tubbo
tubbo commented Dec 24, 2011

Interesting, we were working on the same thing at the same time!

I like how your patch uses an onCreate callback function rather than assuming that creation is going to be handled by an ajax call. This allows one to implement more validation in the middle or even store tokens in localStorage.

@ches ches commented on an outdated diff Dec 30, 2011
src/jquery.tokeninput.js
@@ -757,6 +764,7 @@ $.TokenList = function (input, url_or_data, settings) {
var cache_key = query + computeURL();
var cached_results = cache.get(cache_key);
if(cached_results) {
+ console.log(cached_results)
@ches
ches added a note Dec 30, 2011

Don't forget this :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ches ches commented on an outdated diff Dec 30, 2011
src/jquery.tokeninput.js
populate_dropdown(query, results);
}
}
}
+ function handleCreation(results) {
+ var displayedItem = {};
+ displayedItem['name'] = input_box.val() + ' ' + settings.createTokenText;
+ displayedItem[settings.tokenValue] = input_box.val();
+ displayedItem['wasCreated'] = true;
+ results.push(displayedItem);
+
+ if($.isFunction(settings.onCreate)) {
@ches
ches added a note Dec 30, 2011

This setting doesn't actually appear to exist -- guessing this was left in from code changed later, or intended for further additions to come.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ches ches commented on an outdated diff Dec 30, 2011
src/jquery.tokeninput.js
@@ -505,6 +507,11 @@ $.TokenList = function (input, url_or_data, settings) {
}
}
+ // For newly created tokens, restore the original name without the text saying it is a new token
+ if(settings.allowCreation && item.wasCreated) {
+ item.name = item.id; // restore the name without the token creation text
@ches
ches added a note Dec 30, 2011

This is incorrect if using tokenValue, resulting in a token named "undefined". I replaced the line with:

(settings.tokenValue == "id") ? item.name = item.id : item.name = item[settings.tokenValue];
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@manuelbernhardt

Thanks @ches for the comments, I added your comments to the pull request

@koblas koblas and 2 others commented on an outdated diff Jun 25, 2012
src/jquery.tokeninput.js
@@ -505,6 +508,11 @@ $.TokenList = function (input, url_or_data, settings) {
}
}
+ // For newly created tokens, restore the original name without the text saying it is a new token
+ if(settings.allowCreation && item.wasCreated) {
+ (settings.tokenValue == "id") ? item.name = item.id : item.name = item[settings.tokenValue]; // restore the name without the token creation text
@koblas
koblas added a note Jun 25, 2012

Unless I'm missing something - this overly complicated.

item.name = item.id
and
item.name = item['id']

Are the same statements in JavaScript - thus doing the testing for (settings.tokenValue == "id") is a no-op.

@ches
ches added a note Jun 25, 2012

...but the point is that settings.tokenValue might not be 'id', it is a setting that can be overridden. In the false case of the ternary, this could reference item['my-custom-JSON-field-name-that-is-not-id'].

FWIW though I had to make some further changes to the code in this pull request for it to function correctly for my need, but though they are small, I haven't rigorously examined whether they are generically working for all combinations of config settings/use cases. That's kind of difficult without unit tests for this plugin, but I'll see about submitting a pull request if I can be reasonably confident.

If it's helpful to others trying to make it work for your own needs, basically I just replaced the displayedItem[settings.tokenValue] = input_box.val(); line in the handleCreation function with an entirely new object attribute, displayedItem['newTokenName'] = input_box.val();, and then replaced the ugly ternary line in question above with item.name = item.newTokenName;. So the question of whether this works for all cases is at least isolated to the new creation feature, it would have no impact on prior plugin functionality.

@koblas
koblas added a note Jun 25, 2012

Guess I didn't finish the thought completely - the idea is that

(settings.tokenValue == "id") ? item.name = item.id : item.name = item[settings.tokenValue];

and
item.name = item[settings.tokenValue]

Are the same, since when settings.tokenValue is "id" then it's item.name = item.id. Less code, fewer tests.

Usually I'd agree. But given that this pull request is 7 months old, has 210 open issues and 66 pull requests, out of which a number of them duplicates the functionality provided by this one, what I'd worry primarily about is whether or not this one might get one day pulled in, before putting any more effort into it.

@ches
ches added a note Jun 25, 2012

Oh sorry @koblas, I misinterpreted. Yes, you're right, I don't know what I was thinking when I wrote that. Like I said, I ended up scrapping that line anyway :smile:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@SlashmanX

Any possible chance to make it so the 'onCreate' callback is only called if you select the token i.e. press 'enter' when '(Create new Token) is showing, right now it fires at every result .

EDIT: Nevermind, I see that you added a 'wasCreated' property to the onAdd method. Good work

Andy Dustman and others added some commits Oct 30, 2012
Andy Dustman Merge branch 'master' into token-creation
Conflicts:
	src/jquery.tokeninput.js
7774451
@farcepest farcepest Merge remote-tracking branch 'loopj/master' into token-creation 0f7a55d
@manuelbernhardt manuelbernhardt Merge pull request #1 from farcepest/token-creation
Token creation branched merged with loopj/master
3529389
@thiagodiniz

+1

@jmjpro
jmjpro commented Nov 3, 2014

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.