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

feat(choices): allow user select group header #1998

Merged
merged 12 commits into from
Oct 18, 2017
Merged

feat(choices): allow user select group header #1998

merged 12 commits into from
Oct 18, 2017

Conversation

DarkIsDude
Copy link
Contributor

User can now select header of group by option to select all item in this group

#1074 old pull request ref

User can now select header of group by option to select all item in this group

#1074 old pull request ref
User can now select header of group by option to select all item in this group

#1074 old pull request ref
User can now select header of group by option to select all item in this group

#1074 old pull request ref
@Jefiozie Jefiozie self-assigned this Jun 5, 2017
@Jefiozie Jefiozie self-requested a review June 7, 2017 13:14
@Jefiozie Jefiozie removed their assignment Jun 7, 2017
Copy link
Contributor

@Jefiozie Jefiozie left a comment

Choose a reason for hiding this comment

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

Can we make a seperate attribute directive for this?
As the uiSelectController already has a lot of functionality in the code i would like to separate some additional features vs the core functionality. Second I think there needs to be some tests made and a working example plunker so that i can have a look at it.

@randdusing
Copy link

It would also be nice if the keyboard navigation supported this as well

@DarkIsDude
Copy link
Contributor Author

Hi, we will use it in production. I must complete some task then I will changes this PR.

@DarkIsDude
Copy link
Contributor Author

@Jefiozie thanks for your review. I just push a new commit to create a new directive (ui-select-header-group-selectable) that must be added on ui-select-choices directive with multiple and groups check.

It's my first PR, just say me if I must improve something.
You can find two comment

  • Don't know why I must do this:
    var $select = select[0];
  • And if it's the good way to find the group:
    var group = $select.findGroupByName(element.text(), true);

How can I create a plunker with my repo to show the result? How can I insert my select.js and select.css?

@randdusing I don't know where I can find the code that catch up keyboard event... Can you help me?

@randdusing
Copy link

I've heard this library is completely dead... so may not be worth investing time into a PR. I'm not exactly sure where the general keyboard handling is, but maybe try to find where DOWN is referenced.

@Jefiozie
Copy link
Contributor

You can use the examples plunkers to fork the default ui-select example. Next copy your local build select.js etc in it and share the url.

The keyboard codes are in the common.js

@DarkIsDude
Copy link
Contributor Author

@Jefiozie here the plunker: http://plnkr.co/edit/Ujr9amwp3hYMHwSgNdMT?p=preview to try it.
For the keyboard, it's a little bit more complicated. Maybe I will try it in an another PR.

Is it ok for you?

@Klinton90
Copy link

Just small request: can we make it optional? I.e. scope: {isEnabled: "<uiSelectHeaderGroupSelectable"}.

@DarkIsDude
Copy link
Contributor Author

DarkIsDude commented Sep 20, 2017

@Klinton90 the new plunker with your option: http://plnkr.co/edit/xtJ0SEGneWijtkc06hw5?p=preview.

@Jefiozie why the build fail?

@Klinton90
Copy link

Many thanks!

@kprabhuvel
Copy link

@DarkIsDude.. Even, I need this feature in my application. When do you think you can resolve and have a PR ready? Do you have a timeline when will it be ready? Please let me know.
-Prabhu

@Jefiozie
Copy link
Contributor

Jefiozie commented Oct 6, 2017

@DarkIsDude Could you change something to test the build? Place the brackets on line 3195 and place it on line 3203

I cannot reproduce it but it looks a bit off. (but maybe its just me 😄

@DarkIsDude
Copy link
Contributor Author

DarkIsDude commented Oct 9, 2017

@kprabhuvel, I work on it when I can, it's not a priority in our team...
I hope it will be ready soon but the build fail and I don't know why... @Jefiozie do you have an idea?

@Jefiozie
Copy link
Contributor

Jefiozie commented Oct 9, 2017

@DarkIsDude no I don't know why it does this. Merged other PR yesterday wasn't any problem there. Looks like something is breaking for Firefox as it doesn't start.

@DarkIsDude
Copy link
Contributor Author

So bad :( ... I just run gulp test on Firefox:
browsers: ['Firefox'],
on line 43 in karma.conf.js

And all pass... I don't know how can I fix it. Someone can help me?

@Jefiozie
Copy link
Contributor

Hi @DarkIsDude could you merge the master in your branch and see if the issue is fixed?
Thanks

@DarkIsDude
Copy link
Contributor Author

@Jefiozie yeah!!! It works. Well done. I'm waiting this PR is merged and after I will work on the keyboard navigation.

@Jefiozie
Copy link
Contributor

Hi @DarkIsDude ,

Could you add a small part on the wiki that this is working from the upcoming version?
LGTM 👍

@Jefiozie Jefiozie merged commit b4bbadf into angular-ui:master Oct 18, 2017
@DarkIsDude
Copy link
Contributor Author

@Jefiozie did it ;).

@Klinton90
Copy link

Found couple related issues. PR is #2069
It would be nice if @DarkIsDude can review my changes in case I broke something.

DarkIsDude added a commit to DarkIsDude/ui-select that referenced this pull request Nov 7, 2017
After merge pr angular-ui#1998, build process doesn't work fine
Copy link

@swarnakarraj swarnakarraj left a comment

Choose a reason for hiding this comment

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

when you have On-select function defined for ui-select. It gets triggered for each item when you click on group.

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.

7 participants