Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Passing label as a property to sb.option component #9

Closed
mkay581 opened this issue Sep 3, 2016 · 19 comments
Closed

Passing label as a property to sb.option component #9

mkay581 opened this issue Sep 3, 2016 · 19 comments
Labels

Comments

@mkay581
Copy link

@mkay581 mkay581 commented Sep 3, 2016

Hey, awesome, awesome package!

I just have one quick question. I see that by default, in order to show a label, the code looks to accept an object as a value, and uses a label-key off of that to display the label in the dropdown. Was there any reason why it doesn't just allow passing a label property into the sb.option component? ie:

{{sb.option value="A" label="Apple"}}
@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Sep 3, 2016

Thanks!
That's what you do? I'm confused 🙂

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Sep 3, 2016

Ah I see, it seems you're looking at the addon's dummy app, whose select boxes are just demos of what you can acheive.

@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Sep 3, 2016

Sorry, I want to display the label in the {{#selected-options}} block upon initial load of the select-box component (i'm passing the same value that matches a value passed in one of the {{sb.options}} blocks), but the callback selected only passes the value property as the first param, not the label. I'm looking at the call made in the code here

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Sep 4, 2016

Could you perhaps put the hbs/js of your select-box in pastebin so I can have a look?

The select box only stores the selected value, so at any one time it is not aware what the label being displayed is - it could be anything in-between the selected <option></option> tags. Hence it is up to the developer to make their own way of rendering a selected-option. This example: (component.js, usage) shows just one way, but there are many... it depends on what how you want to interact with the select box and what sort of data you will be selecting from it.

@amk221 amk221 added the question label Sep 4, 2016
@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Sep 4, 2016

Yeah I can do a jsfiddle example demonstration for you...

Just off-hand though, I can provide a little more clarity.

The select box only stores the selected value, so at any one time it is not aware what the label being displayed is

Yeah, definitely. So my question now is, if the select-box component doesn't care about the label or even consider it, why is it passed into the select.option component, shown in the initial examples on the README? It makes me think that the select-box uses the label in some way.

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Sep 4, 2016

This:

{{#api.option value=1}}One{{/api.option}}

will render...

<option value=1>One</option>

Whereas,

{{api.option value=1 label='One' }}

is just an alternative syntax. The select box on the other hand, still knows nothing about this.

(Perhaps the sb. prefix made you think you were telling the select what the label was? when in fact, you are working with the select box's 'api' for rendering an option)

Don't worry about a fiddle/twiddle, I don't think you can include Ember addons into them yet. I'd be curious to see your code at least, maybe I could help some more...

@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Sep 4, 2016

Yeah I understand that those three options are the same. I guess I'm wondering, why even allow the label in the second one:

{{api.option label='One' value=1}}

You're just yielding the contents of the sb.option block (as demonstrated by your first example above), so it may be less confusing to avoid passing in a label attr that the component never uses. Due to my confusion, I was spending time on trying to figure out why the label attr I was passing into my select-option wasn't being passed back into the on-select callback so that I could deal with it.

Am I making any kind of sense? lol

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Sep 4, 2016

Ah, understood... so in me adding the ability to use a label attr, it set up your expectations of extra functionality that doesn't exist. Which is fair!

For the faux-select boxes, a label can be more than just a string.

e.g. Imagine this:

{{sb.option value='Foo' label='Label goes here'}}

Outputting...

<div class="my-select-box-option">
  <span>Label goes here</span>
  <span>&raquo;</span>
</div>

Then imagine the same, but without the ability to pass in a label attribute:

{{#sb.option value='Foo'}}
  <span>Label goes here</span>
  <span>&raquo;</span>
{{/sb.option}}

...how is the select box author supposed to know what your label is: $label = this.$('?');

Perhaps it would be sensible to remove this capability from the native select box?

@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Sep 4, 2016

how is the select box author supposed to know what your label is

the author is the one passing in the label, so they should know what the label is.

let fruits = Ember.A([
{ value: 'A', label: 'Apple'},
{ value: 'P', label: 'Pear'} 
])
{{#select-box on-select=(action 'onSelect') as |sb|}}}
    {{#each fruits as |fruit|}}
        {{#sb.option value=fruit}}{{fruit.label}}{{/sb.option}}
    {{/each}}
{{/select-box}}

Then their onSelect action handler will get passed the entire selected fruit object which now has a label property on it. So they can display it correctly in their selected-option block.

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Sep 4, 2016

If you want the label attribute removing make a PR! I personally like the syntax

@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Sep 4, 2016

But I feel that if you're already accepting a label property (decoupled from the value object), you should pass that back in the on-select callback, or at the very least pass all the attrs that I'm passing into the select-option component.

let fruits = Ember.A([
{ value: 'A', label: 'Apple'},
{ value: 'P', label: 'Pear'} 
])
{{#select-box on-select=(action 'onSelect') as |sb|}}}
    {{#each fruits as |fruit|}}
        {{sb.option value=fruit.value label=fruit.label}}
    {{/each}}
{{/select-box}}

Then my onSelect action should be called with both the value and the label (or at least the entire attrs object, I'm passing into the select-option component). I guess what I'm getting at is, the callback should give me all of the data I'm passing back into it. If not, I'll have to map everything myself in the callback. What do you think?

@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Sep 4, 2016

If you want the label attribute removing make a PR!

lol okay I was actually going to, but just wanted to get your take on this first.

@amk221 amk221 added the wontfix label Dec 6, 2016
@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Dec 6, 2016

Hey, I hope you don't mind, but I'm going to close this - my primary reason being that the select box is for selecting values, not labels.

@amk221 amk221 closed this Dec 6, 2016
@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Dec 7, 2016

No problem. I've been quite busy, so haven't had a chance to submit a PR for the change to clarify the syntax. But will do once I get some down time. Thanks for your time.

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Mar 4, 2018

In v5 (coming soon) I have phased out the label attribute to align better with a native select box.

You should be able to do this:

this.set('models', [{
  id: 1,
  name: 'Foo'
}, {
  id: 2,
  name: 'Bar'
}, {
  id: 3,
  name: 'Baz'
}]);
actions: {
  selected(value) {
    this.set('selectedValue', value);
  }
}
{{#select-box value=model on-select=(action "selected") as |sb|}}
  {{#sb.selected-option}}
    {{selectedValue.name}}
  {{/sb.selected-option}}
  {{#each models as |model|}}
    {{#sb.option value=model as |o|}}
      {{o.value.name}}
      {{! same as model.name, but will resolve the value if a promise }}
    {{/sb.option}}
  {{/each}}
{{/select-box}}
@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Mar 4, 2018

haha that is awesome! And it makes this API much more understandable. Was it a typo? or did you mean...

{{#each models as |model}}
    {{#sb.option value=model as |o|}}
      {{o.name}}
    {{/sb.option}}
  {{/each}}

?

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Mar 4, 2018

^updated

@mkay581

This comment has been minimized.

Copy link
Author

@mkay581 mkay581 commented Mar 4, 2018

Oh.... you expect for the consumer to asynchronously get each value's name? On render? I've seen dropdowns asynchronously fetch a set of options from a server but never a dropdown that would have asynchronous behavior at the option level.

Edit: typo

@amk221

This comment has been minimized.

Copy link
Contributor

@amk221 amk221 commented Mar 4, 2018

No work for the consumer, the select box should just handle everything.

Perhaps this makes it clearer:

{{#select-box value=promise}}
  {{#each promises as |promise|}}
    {{#sb.option value=promise as |o|}}
      {{promise}} {{! Would render [object Object], obviously not very useful }}
      {{o.value}} {{! Would render the resolved value }}
    {{/sb.option}}
  {{/each}}
{{/select-box}}

When an option is selected, instead of receiving <Promise>, you will get the result of the resolved promise, which I assume would be the expected behaviour?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.