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

[WIP] Tagging with ember data support #92

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

koemeet
Copy link

@koemeet koemeet commented May 7, 2015

This fixes issue #7

I am currently working on supporting tags in this component, while also supporting ember data. You can pass in an ember data record array (DS.RecordArray) instance, and it will create new tags based on the
type of the recordarray, almost magic.

This PR is still a WIP and is still made with only ember data in mind, any feedback is welcome!

@iStefo Could you review this, note this is still a WIP, but would like to know your ideas on this.

Currently, the following settings work as you would expect:

Option Type Info
tags bool This is required to enable tagging
content Array This can also be an ember data record array
value Array This can also be an ember data record array, new records are appended automatically. When a new tag is deleted, the record is unloaded from the store.
optionLabelPath string This will be mapped to the field name of the ember data model too, this would be something like title or name.
optionIdPath string Set this the same as the label to have unique tags

Example usage (you could leave out content here if you don't want to preload some tags):

{{select-2
    tags=true
    content=model.tags
    value=model.post.tags
    placeholder="Begin typing tags for this post"
    optionLabelPath="name"
    optionIdPath="name"
}}

TODO

  • Test this functionality for both ember data objects and for simple objects
  • Fix bug where the query filters new tags with the same name as an existing tag correctly
  • Complete examples/documentation
  • Add actions createTag and removeTag that can override default behaviour

I am currently working on supporting tags in this component, while also
supporting ember data. You can pass in an ember data record array
(DS.RecordArray) instance, and it will create new tags based on the
`type` of the recordarray.

This PR is still a WIP, any feedback is welcome!
return (e === undefined) ? null : get(e, optionIdPath);
var id = (e === undefined) ? null : get(e, optionIdPath);
if (self.get('tags')) {
return id.toLowerCase();
Copy link
Author

Choose a reason for hiding this comment

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

I am forcing the lowercase of the ID here, since you don't want tags that only differ in uppercase or lowercase and are still added.

Copy link
Owner

Choose a reason for hiding this comment

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

Seems fine with me, especially since this only applies for tagging=true 👍

@koemeet koemeet changed the title [WIP] Tagging support with ember data support [WIP] Tagging with ember data support May 7, 2015
var record;
if (e.added instanceof DS.Model) {
record = e.added;
} else {
Copy link
Author

Choose a reason for hiding this comment

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

I need to check the type of the value property here, so it can both support ember data record arrays and simple objects.

@iStefo
Copy link
Owner

iStefo commented May 7, 2015

Thanks for the work you put into this! I like how you aim for a high code quality and test coverage, it is great. I added some comments to the code, but I'm already very happy with the direction this is going.
This will be the 1.4.0 feature!

@koemeet
Copy link
Author

koemeet commented May 7, 2015

@iStefo Good to hear 👍 I am sure many people will like this feature. I think this will be the only library that has ember data support build in for creating tags. Without the need of implementing factory actions for every select box. I will work on your comments and refactor my code some more.

Also, I think it is nicer to name the option tags to create. So you configure it like this:

{{select-2
    create=true
    placeholder="Begin typing tags for this post"
    content=model.tags
    value=model.post.tags
}}

A small change, but I think it fits better.

@iStefo
Copy link
Owner

iStefo commented May 7, 2015

Actually, I'm not sure about naming this option create, because the behaviour is what most people would understand as tagging (and also select2 iteself calls it tagging). What do you think about createTags=true? It's a little verbose but also states the suggested use case for the option.

@koemeet
Copy link
Author

koemeet commented May 7, 2015

@iStefo Ah, np, will leave it as tags (the same as select2). Too abstract is sometimes bad I guess.

ember objects instead of ember data models. This is automatically
determined by this component.
@koemeet
Copy link
Author

koemeet commented May 7, 2015

@iStefo I made some good progress and expanded the examples page. It currently uses the matcher to prevent creating new tags that already exists. Because I use the matcher is also handles diacritics. Also added support for plain ember arrays & objects. What do you think of the examples so far, is it enough?

Also, how should we handle optionIdPath? It is kinda ugly now to declare both of them in order to make it work as expected. We can force it to be the same as the optionLabelPath for tags. Since the label is also the ID for tags in the terminology of select2, right?

@iStefo
Copy link
Owner

iStefo commented May 9, 2015

I checked out your branch and fixed the example code indentation (general indentation for the examples.hbs is still a mess, though). How can I incorporate my changes to the PR? PR your forked repo? Sounds unnecessarily complicated though, should I send you my .patch?

@shunchu
Copy link

shunchu commented May 14, 2015

Wow. This is awesome Thank you!

<p>When one select2 components needs special styles, set the <code>cssClass</code> binding to a string
containing a single css class. It will be assigned to the <code>.select2-container</code>, which is the
input in closed state, as well as to the <code>.select2-drop</code>, which is the dropdown menu.</p>
<p><b>Warning:</b> Don't get to fancy! These classes will not update when the binding updates, they have to
Copy link

Choose a reason for hiding this comment

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

too

@abobwhite
Copy link

Hey guys - what's the latest on this? I took a look at the diff and tried to apply some of the Ember Data changes to my local copy but can't figure out where is the correct spot with ember addon. As for this PR, why are we limiting Ember Data support to only when "tags" are being used? Shouldn't I just be able to use a multiple=true with selection=model.myHasManyProperty ? I wasn't aware until now that ember-select-2 did not already support Ember Data officially - which surprised me. I have been very stuck because of ember-select-2 causing Ember to throw an error trying to set a read-only hasMany relationship vs addObject and removeObject....

Let me know if I can help or provide more info as this is a blocker for me right now.

@fullofcaffeine
Copy link

+1 I also need this functionality and was glad I found out someone already worked on it, but it's been a while already, is it ready to be merged? Any ways I could help?

@fullofcaffeine
Copy link

Hmm, I see select2 (and select2-ember) already supports multiple=true for a similar functionality. What's the difference here?

@@ -505,6 +590,9 @@ var Select2Component = Ember.Component.extend({
this._select.select2("val", value);
} else {
// otherwise set the full object via "data"
if (value instanceof Ember.ArrayProxy) {
Copy link

Choose a reason for hiding this comment

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

I had to change Ember.ArrayProxy to DS.ManyArray to get this to work as expected. I'm on Ember 1.13.7 and Ember Data 1.13.8, and when I use the hasMany relationship as value, this is false, and my values aren't properly displayed. Obviously this would break for those not using Ember Data.

@joaoribeirost
Copy link

Are these changes usable when we install the addon? I'm trying to get it to work but it seems i still can't add new tags.
My code is like this:

{{select-2
    content=tag_list
    optionValuePath="name"
    optionLabelPath="name"
    optionIdPath="name"
    value=selectedTags
    multiple=true
    createTags=true
    placeholder="Choose some Tags"
 }}

Is there something i'm doing wrong?

@valtlfelipe
Copy link

Any update on this? I'm using Ember v1.13.3 with Ember Data v1.13.5 and it is not working.
I'm getting this warning in console:

WARNING: select2#initSelection was not able to map each "id" to an object from "content". The remaining keys are: 1,abc. The input will be disabled until a) the desired objects is added to the "content" array or b) the "value" is changed.

It appears that the new value is not getting added to the content property, just to the value, which is causing this error.

@shunchu shunchu mentioned this pull request May 10, 2016
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.

9 participants