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

controlled input on select tag requires both select value AND option selected #37

Open
modernserf opened this issue Jul 10, 2016 · 14 comments

Comments

@modernserf
Copy link

modernserf commented Jul 10, 2016

More of a documentation request than a feature request -- this is how to do a "controlled" (in the React parlance) version of a select tag:

const select = (options, value) =>
    yo`<select value="${value}">
        ${options.map((o) =>
            yo`<option value="${o.value}"
                ${value === o.value ? "selected" : ""}>
                ${o.label}
            </option>`
        )}
    </select>`

Note that you need to both set a value on the <select> tag and set the selected attribute on the selected <option>.

@shama
Copy link
Collaborator

shama commented Jul 10, 2016

It looks like selected wasn't included in the list of boolean attributes, that is now fixed.

Give this a try:

const select = (options, value) =>
    yo`<select>
        ${options.map((o) =>
            yo`<option value="${o.value}"
                selected="${value === o.value}">
                ${o.label}
            </option>`
        )}
    </select>`

More documentation on boolean attributes would be good!

@modernserf
Copy link
Author

That's still not working for me. I'll put together a demo when I get a chance.

@shama
Copy link
Collaborator

shama commented Jul 10, 2016

@modernserf Try npm cache clean && rm -rf node_modules && npm i to get the latest dependency tree.

@modernserf
Copy link
Author

$ npm ls bel

└─┬ [email protected]
  └── [email protected]

this should work?

@shama
Copy link
Collaborator

shama commented Jul 10, 2016

Hmm yep, 4.4.3 should work. A demo of what's not working would be great then. Thanks!

@timwis
Copy link
Contributor

timwis commented Jul 10, 2016

FYI, this works for me

/**
 * Create a <select> element
 * @param {string[]|Object[]} items List of value strings or objects with value and label properties
 * @param {string} selected Value of selected item
 * @param {Object} attributes Object of attributes to apply to <select> element
 */
module.exports = ({ items = [], selected, attributes = {} }) => {
  return html`
    <select ${attributes}>
      ${items.map((item) => {
        const optAttributes = {
          value: item.value || item
        }
        if (selected && selected === optAttributes.value) {
          optAttributes.selected = 'selected'
        }
        const label = item.label || item
        return html`<option ${optAttributes}>${label}</option>`
      })}
    </select>`
}

@modernserf
Copy link
Author

@shama
Copy link
Collaborator

shama commented Jul 11, 2016

Thanks! I'll take a look and see if I can find out why it's not working. I wonder if it's related to Babel parsing the template strings?

The main reason I prefer the syntax that supplies the attributes <option selected="${isSelected}"> over an expression <option ${attrs}> is because we can parse the HTML attributes if they're not in an expression. This allows us to make better optimizations when converting the template literal into elements.

@timwis
Copy link
Contributor

timwis commented Jul 11, 2016

Agreed @shama, actually the only reason I used an expression is because selected="false" still renders as selected. Though I may be thinking of checked. Basically, there's no false value technically for them in html

@fjeldstad
Copy link

I've also found that both select.value and option.selected is needed to be able to programmatically select an option (such as when the options are rendered first, and then an async action is selecting one of the options).

@fjeldstad
Copy link

I guess is has to do with yo-yo copying the value of the existing select element to the new one (https://github.com/maxogden/yo-yo/blob/master/index.js#L29).

@yoshuawuyts
Copy link
Collaborator

yoshuawuyts commented Oct 6, 2016

@Hihaj I'm not sure I follow: what issue are you experiencing?

@fjeldstad
Copy link

@yoshuawuyts I was just making the same observation as @modernserf in the original question; that you need to use both (select.value + option.selected) in order to control which option should be selected via some other action than the user actually picking it in the dropdown. For example if the <select> first renders with a couple of options and you (a little bit later) want to set one of the options as selected (based on some logic).

@yoshuawuyts
Copy link
Collaborator

@Hihaj thanks; that makes sense - feel we should address this indeed ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants