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

Improve performance, add filter emit event #1460

Closed
wants to merge 3 commits into from

Conversation

Avien
Copy link

@Avien Avien commented Feb 25, 2016

Menu is very slow when trying to search on 100 items
Also closing the menu has a significant timeout delay
Removed the newly animations just added when closing the menu
Removed $timeout when opening the menu

Added the ability to emit event when items get filtered so I can react to that in my component and apply some basic logic , i.e.: check if already selected items are still valid to display

@buremba
Copy link

buremba commented Feb 25, 2016

You need to modify files in src/* directory, dist/* directory is automatically built in release phase.

@Avien
Copy link
Author

Avien commented Feb 25, 2016

Yep , your right, sorry
Updated 2 files in the src directory
Thanks

@user378230
Copy link
Contributor

I don't think these should be merged as is... they're performance improvements at the cost of intentionally added functionality.

  1. Instead of removing animation support can't you just disable it on the container?
  2. $select.setActiveItem is no longer called
    • As far as I can see it was removed and not added anywhere else, which would be a breaking change.

@Avien
Copy link
Author

Avien commented Feb 25, 2016

I needed a specific behaviour which suits me fine
Removing the setActiveItem solved a very problematic slow down in the menu which did not exist on 0.13.2
I only need it to work with angular 1.5 and did not see any problems after removing this method

You can ignore this pull request if you want but please be aware that this method creates a performance issues

As for the emit filter event I added, I expected the component to remove an already selected items in the model if they are no longer exist in the new filtered list (affected by the 'filter' property mentioned in the documents) but it doesn't so I needed the event to do it myself

Thanks

@wesleycho
Copy link
Contributor

Changes to dist need to be undone, and this needs rebasing.

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

Successfully merging this pull request may close these issues.

4 participants