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

De-duplicate entries over labels #76

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

Conversation

pritambaral
Copy link

If the input model has multiple rows with the same item-label, don't treat them as different entries.

Example (we want to filter by engine):

$scope.outputBrowsers = [
    {   icon: "[...]/firefox.png...",  name: "Firefox",  engine: "Gecko", check: true   }
    {   icon: "[...]/chromium.png...", name: "Chromium", engine: "Blink", check: true   }
    {   icon: "[...]/opera-32.png...", name: "Opera",    engine: "Blink", check: true   }
]; 

@alexandernst
Copy link
Owner

Hi!

I'm not really sure I understand what your changes are supposed to do. Are you dedup-ing on the engine key (in this particular case)? As in: are you using the engine key as an ID?

@pritambaral
Copy link
Author

Close. I need to use the engine key in item-label.

snap3
snap4

De-duplicate data that is shown in this widget: this includes the
clickable, checkable items and also the button label. This probably
needs a refactoring, especially the $scope._walk method
@alexandernst
Copy link
Owner

My boss just reassigned me to this project, so I'll start working on v6, which will be a complete rewrite with performance in mind.

My tasks are (literal project requirements)

  • at least 100% speed improvement
  • no missing features from current version
  • make it easier to implement new features
  • have a look at current PRs

This is the only PR so I'll try to understand better the need of this before deciding if/how to implement it.
I do see what you try to achieve there, and I do see it like a desirable feature, anyways, I kind of disagree with the implementation (or at least I have doubts about it).

Wouldn't it be easier to just make the directive accept arrays as values? That way you could just pass

[
    {
        name: "Blink",
        value: ["Chrome", "Chromium"]
    },

    {
        name: "Gecko",
        value: ["Firefox", "Iceweasel"]
    },

    {
        name: "Trident",
        value: "Internet Explorer"
]

Or said with other words, why should we implement deduping rather than just accepting all types of data as values?

@alexandernst
Copy link
Owner

By the way, sorry for not REing before, I have 0 time until now.

@pritambaral
Copy link
Author

What you suggested could be used, but I see it more as a hack. It breaks the simplicity of use; where previously I could simply take the output of angular-multi-select and wire up my views with a single ng-repeat, now I'd have to either nest ng-repeats or de-aggregate possible arrays in the output.

@alexandernst
Copy link
Owner

Agreed. I'll look how to implement this in v6.

@alexandernst
Copy link
Owner

@pritambaral I haven't forgot about this one. Just pushed the initial v6 code. Will get to this very soon.

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.

2 participants