feat(removeSelected): Make removeSelected configurable #1567
Conversation
...at the cost of potential unnecessary watches when dropdown is closed. But the premise is that, until the resulting errors are fixed, the preference is to avoid the errors for now. Closes angular-ui#1355
Expose the removeSelected property, and adjust isDisabled behavior so that items that would have been removed are disabled instead. Work in progress: This broke some tests
|
@dondi The code itself looks ok but there aren't any unit tests. Can you add some and I'll review again? Thanks! 👍 |
|
Ack, sorry! I had fixed some unit tests that broke due to this change but then got distracted that day and totally forgot about adding new ones. Sorry, my bad, I will notify you when I’ve put something up. |
|
@dondi Is this PR still something you're working on recently? The functionality this PR contains is something I'm looking for as well, so I could take over the PR if it doesn't fit in your schedule =) |
Working to round out angular-ui#1567
Toward merging of angular-ui#1567
|
Thanks very much for the offer @thomasmiddeldorp 👍 As it turns out, I was chipping away at it and managed to have enough time to wrap things up this morning. Had a learning curve to climb there for a bit. I just pushed both unit tests and a demo—hope it's what you're looking for. |
|
Almost perfect @dondi! Can you just squash down into a single Once that's done I'll merge 👍 Useful reference: Commit Message Guidelines |
|
Ah OK, I'll look into that…I presume the "merge from master" commit that is in between there is part of this noise. Yeah I can see how that would be problematic. Would it work better if I create a fresh branch off upstream then place my changed files there, do a single commit, and issue a new pull request? |
|
OK, I have a new pull request using a fresh branch that is updated from upstream. Hope that will be OK. New pull request: #1622 |
|
You've opened the new one now but you can just force push to your existing branch after squashing, github would have automatically updated this PR with the new single commit. :-) |
|
Yes, I intended to do that at first, but after looking more closely at the overall commits, there were a couple that were no longer valid plus the merge from master commit was in between commits of mine that I wanted to keep, with potential dependent changes in between. So in the end it was more straightforward to start fresh. |
Expose the
removeSelectedproperty, and adjustisDisabledbehavior so that items that would have been removed from amultipleui-select element because they are currently selected are disabled instead.Sample behavior:
In the existing version,
idwould not have appeared in the dropdown because it is currently selected. This pull request adds the optionremoveSelectedso that, if set tofalse, selected items still appear in the dropdown but are disabled (like in the screenshot).