Feature/new new chips #344
Conversation
|
Howdy folks! It's me again 😄 I would like to get this in master before I need to do another rebase so if anyone has any objections just let me know and I'll try to turn them around ASAP. As I mentioned in my previous attempt #267 the autocomplete is broken but that is not because of what I've done. That seems to be broken on master and I would like to tackle that after this gets merged if at all possible 😄 |
|
See the CONTRIBUTING.md for running jscs locally. The failures I see are style issues. That guide also has some info that you may or may not already know about how we are trying to do component ports to 1.0. |
|
thanks @DanChadwick 👍 Re: component ports, I thought I followed a previous guilde (or maybe it was copying early implementaitons) but is there something particular (Stylesheets, Naming, Attributes etc.) that you think I should re-look at? |
|
jscs should be running as part of the test suite since we've upgraded to ember-cli 2.4 |
|
I didn't have anything specific in contributing. I just thought it would be helpful. In fact, maybe you have things you think should be added it it?? |
|
@miguelcobain - I don't seem to see identical behavior between running tests locally and on travis. I can't recall what, but I remember seeing jscs "errors" caught by atom that weren't caught by |
|
@DanChadwick ah cool, I thought you saw something horrific in my "style" and you were trying to be polite 😂 Looks like the tests are passing now. Let me know if there is anything I still need to change before we can merge 👍 |
| @@ -217,7 +218,7 @@ export default Ember.Component.extend(HasBlockMixin, { | |||
| shouldHide: Ember.computed.not('isMinLengthMet'), | |||
|
|
|||
| isMinLengthMet: Ember.computed('searchText', 'minLength', function() { | |||
| return this.get('searchText').length >= this.get('minLength'); | |||
| return this.get('searchText.length') >= this.get('minLength'); | |||
There was a problem hiding this comment.
Does this fix a bug because searchText is sometimes not a string? We are trying to isolate changes to autocomplete to minimize merge conflicts, since there is already WIP in another PR.
|
I made a few comments in anticipation of an eventual review by @miguelcobain. I would love to see this merged when ready. It doesn't seem to be as complete as the Angular implementation -- or at least the demo isn't as extensive -- but it seems to me to set a good baseline for basic use. There are other components which maybe are ahead of it in the queue. For example, tabs is currently in progress, and we would very much like to have that working so that we can use tabs for documentation purposes in the demo app. Are you planning on tackling autocomplete after this? |
mansona
force-pushed
the
feature/new-new-chips
branch
from
a0cfad6 to
630fad7
Compare
|
Ok so I've made most of the changes that you suggested @DanChadwick and reverted the stuff that you pointed out re: autocomplete. I am still in a bit of bind here... the autocomplete implementation doesn't work but this is most likely because autocomplete is current broken in this branch. How far down the line is the new autocomplete work? I could really do with a modernised autocomplete implementation to finish off this PR. Also what is your recommendation @miguelcobain re @DanChadwick 's question about the md-close icons #344 (comment) |
|
Hey Folks, I know you're all probably super busy. I am just checking if anyone has had a chance to look into this PR? Is there anything I can do to move it along or help out? |
|
I'm also happy to help progress this PR - please let me know if there's anything I can do to get this moving along! |
|
There are a lot of work related to paper-autocomplete/paper-select/paper-menu happening right now on paper-menu branch. Hopefully very soon we'll have a stable version of paper-autocomplete merged into the master that should help finalize paper-chips impl. |
Ttrigger add/remove contact actions properly in Chips demo
|
I've rebased the feature/new-new-chips branch onto the current (as of yesterday) paper-menu branch, made it work with the paper-autocomplete from that branch (based on ember-power-select) and added some more polish and missing functionality (such as keyboard navigation). I'll hold off submitting a new (new new) PR until the paper-menu stuff lands and I've done a final rebase on that - I see there have already been more commits since I rebased; for now, feel free to take a look at https://github.com/pauln/ember-paper/tree/eps-chips if you want to see how it's progressing. |
|
@pauln i would recommend submitting your PR now as a work in progress, that way we can close this one 👍 |
|
@pauln you can make a PR to the paper-menu branch and then we could close this one. |
Second update of the Chips functionality. Continuation of #267