Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(select): add multiple attribute #13119

Closed
wants to merge 1 commit into from

Conversation

Puigcerber
Copy link
Contributor

In the documentation of the select directive there is an example of multiple selection, but the multiple attribute is not listed as an argument, so I think it can be a bit confusing.

@gkalpak
Copy link
Member

gkalpak commented Oct 26, 2015

I think this is because multiple doesn't have a special meaning in Angular (as far as the user is concerned at least). It is standard HTML for allowing multiple options to be selected. (Of course Angular does certain things under the hood to support multiple values, but that is not relevan here imo).

I am not sure if this needs to be documented.

@Puigcerber
Copy link
Contributor Author

I know. But in the same fashion both name and required are added to the documentation. Should we maybe write a note saying that all the HTML5 attributes are valid?

@gkalpak
Copy link
Member

gkalpak commented Oct 27, 2015

name and required are documented because they carry extra semantics for Angular (e.g. name is the property under which the control is registered with its parent form and required sets the corresponding error key in the control's and its parent forms' $error objects).
Also note that other HTML5 attributes (e.g. read-only or disabled etc) are not documented either.

Anyway, since multiple changes the bound value (into an array), we could document that.
Would you like to update the PR, explaining that adding multiple will result into an array of values as model-value ?

@Puigcerber
Copy link
Contributor Author

That makes perfect sense. I'll update the pull request. Thanks @gkalpak!

@gkalpak
Copy link
Member

gkalpak commented Oct 27, 2015

Thx @Puigcerber ! Feel free to ping me when you're done, so I can take another look.

@Narretz Narretz added this to the Backlog milestone Oct 30, 2015
Add multiple parameter to the documentation of the select directive.
@Puigcerber
Copy link
Contributor Author

Hey @gkalpak! I just updated it.

@gkalpak gkalpak closed this in 865f606 Oct 30, 2015
gkalpak pushed a commit that referenced this pull request Oct 30, 2015
Add the `multiple` attribute to the documentation of the select directive.

Closes #13119
@gkalpak
Copy link
Member

gkalpak commented Oct 30, 2015

Merged. Thx @Puigcerber 👍
(Backported to v1.4.x as 5758d73.)

@Puigcerber Puigcerber deleted the docs-select branch October 31, 2015 17:54
gkalpak pushed a commit to gkalpak/angular.js that referenced this pull request Nov 23, 2015
Add the `multiple` attribute to the documentation of the select directive.

Closes angular#13119
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants