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

fix(uiSelectHeaderGroupSelectableDirective): do not issue error if feature is disabled; apply selectable feature after options list changed/filtered; do not select unnamed group #2069

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

Conversation

Klinton90
Copy link

@Klinton90 Klinton90 commented Oct 24, 2017

Trying to use new feature on my project. Would like to fix issues as soon as I find it.

  1. We use wrapper around ui-select, so we always need uiSelectHeaderGroupSelectableDirective directive to exist on template even when ui-select is used for single/no-groups mode. In current implementation, if directive is disabled and other conditions do not pass, console.error() will be executed. That fix prevents error message to be shown for disabled directive.

  2. If list of available options is filtered on the fly (based on $select.search, e.g. ng-repeat="options | filter: $select.search"), not all group headers receive ui-select-header-group-selectable class, as DOM is updated after enableClick() function runs.

  3. Do not add click listener to unnamed group

tymofieo added 2 commits October 24, 2017 16:30
…ature is disabled, but directive exists on template
@Klinton90 Klinton90 changed the title fix(uiSelectHeaderGroupSelectableDirective): do not issue error if feature is disabled fix(uiSelectHeaderGroupSelectableDirective): do not issue error if feature is disabled and apply selectable feature after options list changed/filtered Oct 25, 2017
@DarkIsDude
Copy link
Contributor

Hi @Klinton90 , look good to me 👍 .

@Klinton90 Klinton90 changed the title fix(uiSelectHeaderGroupSelectableDirective): do not issue error if feature is disabled and apply selectable feature after options list changed/filtered fix(uiSelectHeaderGroupSelectableDirective): do not issue error if feature is disabled; apply selectable feature after options list changed/filtered; do not select unnamed group Oct 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants