Method refresh not working

[nvtin]

I am using this plugin. In view I call <abp-tags-input element.bind="tag" value.bind="keywords"></abp-tags-input> And in viewModel I have this.keywords = ['word1', 'word2'] actually I add @bindable tag; It display tags so great when the first time I go to page. but after I did a change the keywords like: this.keywords = ['other-value'] and call this.tag.methods.refresh(); But nothing change from input tag even the value for input is updated. checked by: $('abp-tags-input input.form-control.au-target').val() => other-value but the tags UI does not update. On browser console, I try to call $('abp-tags-input input.form-control.au-target').tagsinput('refresh') but it's not work anymore. What thing I am doing wrong or is it a bug?

Thanks!

[ghiscoding]

Thanks for reporting the issue, I might not have tested it the way you tried. So I can't say for sure if it's an issue or not. However if you try doing like in the client samples that I provided. You can do it this way, multiple steps.

this.tag.methods.removeAll();
this.tag.methods.add('other-value');

oh wait, having another look at my plugin code and I can see that I didn't implement a valueChanged, so yeah I can understand why it doesn't work. I'll have to look at implementing it eventually, I'll see if I can do that in the coming week or so, in the mean time you can use the code I posted just on top.

[ghiscoding]

Looked at this issue and I see that if you want to use an array of strings, you should use the multiple as shown in the 2nd example on Bootstrap-tagsinput

True multi value Use a as your input element for a tags input, to gain true multivalue support. Instead of a comma separated string, the values will be set in an array. Existing elements will automatically be set as tags. This makes it also possible to create tags containing a comma.

ghiscoding commented 7 years ago

However this doesn't fix the main problem which is to replace/refresh the values. What I then found is that the tagsinput('refresh') doesn't really work as shown in this open issue #98 that is quite old (2014).

Another approach is to use removeAll and then add with a merged by comma string. Something like this:

let mergedValues = ['word1', 'word2'].join(',');
this.tag.methods.removeAll();
this.tag.methods.add(mergedValues );

That should work but is not very convenient, I might be able to change the code to handle that but I would have to use something else than value.bind, because if I do the removeAll and the join within the valueChanged, it calls it recursively multiple times which is not good at all.

I'll experiment more with it, might have to release a major release if that is the case

ghiscoding commented 7 years ago

So I got something working but I want some feedback from your side as this would be a major change. As described earlier, because of bootstrap-tagsinput - open issue #98, calling a .tagsinput('refresh') just isn't enough.

If you stick with current version, you can do the following and it works (as described in previous post): 1- add the multiple in your View

<abp-tags-input multiple element.bind="tag" value.bind="keywords"></abp-tags-input>

2- call .removeAll() and then .add()

let mergedValues = ['word1', 'word2'].join(',');
this.tag.methods.removeAll();
this.tag.methods.add(mergedValues );

However this is cumbersome and what we want at the end is to simply update the variable and then see the UI refresh the tags. To do just that, I can move the code shown above and add it the binded changed value. An issue arise while doing it inside a valueChanged, this makes a recursive call (because calling a removeAll and .add changes the value attribute which calls the valueChanged and we never get out of the loop).

If instead rename the value.bind to something else, let say model.bind (or even tag.bind) and then call the modelChanged it works and it's not recursive (it still changes the value attribute and still calls a valueChanged but we are not dealing with it anymore). This mean I would have to remove the value.bind completely to avoid recursion calls. The code I need to add to my plugin is this

modelChanged(newValue, oldValue) {
    if (newValue !== oldValue && newValue && this.domElm) {
      if (Array.isArray(newValue)) {
        let mergedValues = newValue.join(',');
        this.domElm.tagsinput('removeAll');
        this.domElm.tagsinput('add', mergedValues);
      }
    }
  }

Any other thoughts? Please provide feedback before I do this major change (probably a 2.x version release for that).

nvtin commented 7 years ago

@ghiscoding Hi guy. Thanks for your responding me for this issue. I've resolved my issue by using 2 methods:

this.tag.methods.removeAll();
this.tag.methods.add(new_values );

it worked. btw for add 'multiple' as you mentioned,

 add the multiple in your View
   <abp-tags-input multiple element.bind="tag" value.bind="keywords"></abp-tags-input>

it doesn't work on my project.

ghiscoding commented 7 years ago

@nvtin I reworked the code and found a way to use valueChanged without recursion, however I might need to ask a question at Aurelia on StackOverflow if the way I am doing it is ok or not. If it works, the plugin will have the same signature and you won't need to do the remove/add yourself (that means removing the 2 lines of code you added).

As for the multiple, I might have read it too fast when I saw it. It's basically used with a <select> not with in an <input>, so it's not valid and won't work with my plugin. So we can forget about that one.

Will hopefully have a new version by the weekend and I'm thinking about doing a minor release in the form of version 1.1.x

ghiscoding commented 7 years ago

Unfortunately, I thought that I had found a way to detect when the value is changed from the outside (like from the ViewModel) but it actually doesn't work. Without this, I can't call the removeAll and re-add them without falling into a recursive call. I posted a StackOverflow question, but so far there isn't much good answer to fix the issue I'm facing with this plugin. If you know a way, please let me know

Here's the StackOverflow question

ghiscoding commented 6 years ago

I believe that I finally fixed the issue by finding a way to distinct valueChanged inside the customElement from the outside ones. This was very important to remove possible recursion in the plugin.

This mean that the following piece of code is no longer necessary since it is now part of the valueChanged of the plugin itself.

// NOT NECESSARY ANYMORE FROM VERSION 1.1.0+
this.tag.methods.removeAll();
this.tag.methods.add(new_values );

You can simply just change your property for an array of strings and it's all good as expected. For example, in my sample I can now call directly :

replaceAllTags() {
    this.post.categories = ['Erlang', 'Python']; // change all categories
}

I release a new semver minor release since it is an important change and might conflict with the workaround you had put in place (that was provided earlier). Please update to version NPM version 1.1.0 and report back if anything is wrong.

Thanks for the feedback

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[ghiscoding]

However this doesn't fix the main problem which is to replace/refresh the values. What I then found is that the tagsinput('refresh') doesn't really work as shown in this open issue #98 that is quite old (2014).

Another approach is to use removeAll and then add with a merged by comma string. Something like this:

let mergedValues = ['word1', 'word2'].join(',');
this.tag.methods.removeAll();
this.tag.methods.add(mergedValues );

That should work but is not very convenient, I might be able to change the code to handle that but I would have to use something else than value.bind, because if I do the removeAll and the join within the valueChanged, it calls it recursively multiple times which is not good at all.

I'll experiment more with it, might have to release a major release if that is the case

[ghiscoding]

So I got something working but I want some feedback from your side as this would be a major change. As described earlier, because of bootstrap-tagsinput - open issue #98, calling a .tagsinput('refresh') just isn't enough.

If you stick with current version, you can do the following and it works (as described in previous post): 1- add the multiple in your View

<abp-tags-input multiple element.bind="tag" value.bind="keywords"></abp-tags-input>

2- call .removeAll() and then .add()

let mergedValues = ['word1', 'word2'].join(',');
this.tag.methods.removeAll();
this.tag.methods.add(mergedValues );

However this is cumbersome and what we want at the end is to simply update the variable and then see the UI refresh the tags. To do just that, I can move the code shown above and add it the binded changed value. An issue arise while doing it inside a valueChanged, this makes a recursive call (because calling a removeAll and .add changes the value attribute which calls the valueChanged and we never get out of the loop).

If instead rename the value.bind to something else, let say model.bind (or even tag.bind) and then call the modelChanged it works and it's not recursive (it still changes the value attribute and still calls a valueChanged but we are not dealing with it anymore). This mean I would have to remove the value.bind completely to avoid recursion calls. The code I need to add to my plugin is this

modelChanged(newValue, oldValue) {
    if (newValue !== oldValue && newValue && this.domElm) {
      if (Array.isArray(newValue)) {
        let mergedValues = newValue.join(',');
        this.domElm.tagsinput('removeAll');
        this.domElm.tagsinput('add', mergedValues);
      }
    }
  }

Any other thoughts? Please provide feedback before I do this major change (probably a 2.x version release for that).

[nvtin]

@ghiscoding Hi guy. Thanks for your responding me for this issue. I've resolved my issue by using 2 methods:

this.tag.methods.removeAll();
this.tag.methods.add(new_values );

it worked. btw for add 'multiple' as you mentioned,

 add the multiple in your View
   <abp-tags-input multiple element.bind="tag" value.bind="keywords"></abp-tags-input>

it doesn't work on my project.

[ghiscoding]

@nvtin I reworked the code and found a way to use valueChanged without recursion, however I might need to ask a question at Aurelia on StackOverflow if the way I am doing it is ok or not. If it works, the plugin will have the same signature and you won't need to do the remove/add yourself (that means removing the 2 lines of code you added).

As for the multiple, I might have read it too fast when I saw it. It's basically used with a <select> not with in an <input>, so it's not valid and won't work with my plugin. So we can forget about that one.

Will hopefully have a new version by the weekend and I'm thinking about doing a minor release in the form of version 1.1.x

[ghiscoding]

Unfortunately, I thought that I had found a way to detect when the value is changed from the outside (like from the ViewModel) but it actually doesn't work. Without this, I can't call the removeAll and re-add them without falling into a recursive call. I posted a StackOverflow question, but so far there isn't much good answer to fix the issue I'm facing with this plugin. If you know a way, please let me know

Here's the StackOverflow question

[ghiscoding]

I believe that I finally fixed the issue by finding a way to distinct valueChanged inside the customElement from the outside ones. This was very important to remove possible recursion in the plugin.

This mean that the following piece of code is no longer necessary since it is now part of the valueChanged of the plugin itself.

// NOT NECESSARY ANYMORE FROM VERSION 1.1.0+
this.tag.methods.removeAll();
this.tag.methods.add(new_values );

You can simply just change your property for an array of strings and it's all good as expected. For example, in my sample I can now call directly :

replaceAllTags() {
    this.post.categories = ['Erlang', 'Python']; // change all categories
}

I release a new semver minor release since it is an important change and might conflict with the workaround you had put in place (that was provided earlier). Please update to version NPM version 1.1.0 and report back if anything is wrong.

Thanks for the feedback