added country dropdown - #878
Conversation
|
I'm going to redo this using the x-select component, display the country as a full name in the select ui. and I also need to add tests. |
|
@rileytaylor you can look to my other open PR for ideas on how to test with selectable. |
0a54d40 to
e41cfde
Compare
| }); | ||
|
|
||
| test('it sets the country strings correctly', function(assert) { | ||
|
|
There was a problem hiding this comment.
Think you can use the existing tests in the select/birth-date
82a1f67 to
57607c7
Compare
|
|
||
| page.render(hbs`{{select/country-select country=country}}`); | ||
|
|
||
| page.country.fillIn('US'); |
There was a problem hiding this comment.
Can you only fillIn with the value and not the full name.
There was a problem hiding this comment.
I'm not sure what you mean by this, isn't US is the value?
There was a problem hiding this comment.
I'm asking if we can pass "United States" instead, more out of curiosity.
There was a problem hiding this comment.
Oh! So the select box says "United States" but the value passed around is US.
|
|
||
| let country = this.get('country'); | ||
|
|
||
| assert.equal(country, 'US'); |
There was a problem hiding this comment.
I would definitely condense all of these down without gaps between lines.
|
|
||
| page.country.fillIn('US'); | ||
|
|
||
| let country = this.get('country'); |
There was a problem hiding this comment.
Prefer get(this, 'prop') and set(this, 'prop'). const { get, set } = Ember;
There was a problem hiding this comment.
Should birth-date also use this? I agree this is better I just didn't do it initially since i didn't see any other tests using it
| export default Component.extend({ | ||
| classNames: ['select-country'], | ||
| countryOptions: [ | ||
| { name:'United States', abbr:'US' } |
There was a problem hiding this comment.
Would change abbr to id. Also would space after each :
|
|
||
| const { | ||
| Component | ||
| } = Ember; |
rileytaylor
added
needs clarification
question
and removed
needs submitter response
labels
| @@ -0,0 +1,10 @@ | |||
| <label> | |||
| Country | |||
| </label> | |||
There was a problem hiding this comment.
I would actually out the label outside so that can be user-defined. Don't need a wrapping div around the select either.
| {{input type="text" name="zip" value=zip}} | ||
| </div> | ||
| <div class="input-group"> | ||
| <label for="country">Country</label> |
There was a problem hiding this comment.
Would add this label back.
0025bc1 to
0518ee6
Compare
|
🙌 |
2 participants
What's in this PR?
This enables the country dropdown and makes it a proper select dropdown. The only option, for now, is 'US'
References
Fixes #864
Progress on: #853