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

Implemented disabled attribute for select_tag form helper #1054

Merged
merged 2 commits into from Jul 1, 2013

Conversation

dariocravero
Copy link

This implements the requested functionality in #872.

You can now tell an option is disabled by setting its third value in the array that describes it as follows:

options = [['Green', 'green1', true], ['Blue', 'blue1'], ['Black', "black1"]]

In the example, Green will be disabled.

I also implemented this for optgroups since they're subject to being disabled too.

If you're using optgroups as arrays, then again, set set the third item on the array:

opts = [
  ["Friends",["Yoda",["Obiwan",2]]],
  ["Enemies", ["Palpatine",['Darth Vader',3]], true]
]

The group Enemies will be disabled.

For the hash, this was a bit trickier since there wasn't an easy way to tell if the last option was meant to be a boolean value or a directive to tell if you were trying to disable or not the option. Therefore, I decided to force you to use a hash {:disabled => true}:

opts = {
  "Friends" => ["Yoda",["Obiwan",2]],
  "Enemies" => ["Palpatine",['Darth Vader',3], {:disabled => true}]
}

Once again, Enemies will be disabled on the example.

Thoughts? If you decide to merge this in, let me know and I'll update the docs accordingly. :)

@ujifgc
Copy link
Member

ujifgc commented Feb 12, 2013

I strongly dislike the API. I think it should be like :disabled => 'green' or :disabled => ['dark', 'light'] inside options for select_tag. This will conform with :selected option.

@dariocravero
Copy link
Author

I thought about it too at the beginning but something put me off from implementing it that way. Sorry, I did this a few days ago and didn't push it until now. I'll review it again.

@nesquena
Copy link
Member

Let's do this...but let's do it in 0.11.X :)

@dariocravero
Copy link
Author

I was thinking about the API for this issue and I reckon that a :disabled option makes sense for disabling options in non grouped items but what about groups? And what about options inside groups? Say we have something like this:

<select>
  <optgroup label=Group>
    <option value=item>Item</option>
  </optgroup>
  <optgroup label="Group 2">
    <option value=item>Item</option>
    <option value=item-2>Item 2</option>
    <option value=group>Group</option>
  </optgroup>
</select>

If we want to disable item, which one should it be? There's nothing preventing someone from doing that.
And what if we want to disable the group Group? Should we have a specific :disabled_groups option?

Perhaps it makes sense to let those corner cases aside?
Thougths? @ujifgc @nesquena

@ujifgc
Copy link
Member

ujifgc commented Mar 13, 2013

The same questions apply to the :selected API.

I think maybe allow select_tag to receive String/SafeBuffer :options_html? 96% of cases will be fine and in edged ones the programmer will build the options himself.

@DAddYE
Copy link
Member

DAddYE commented Mar 20, 2013

@dariocravero status?

@dariocravero
Copy link
Author

Goes into 0.11.x. It needs further discussion.
On Mar 19, 2013 9:07 PM, "DAddYE" notifications@github.com wrote:

@dariocravero https://github.com/dariocravero status?


Reply to this email directly or view it on GitHubhttps://github.com//pull/1054#issuecomment-15151570
.

DAddYE added a commit that referenced this pull request Jul 1, 2013
…t-options

Implemented disabled attribute for select_tag form helper
@DAddYE DAddYE merged commit c409e42 into master Jul 1, 2013
@DAddYE
Copy link
Member

DAddYE commented Jul 1, 2013

@dariocravero merged, take care of this since is quite old 👍

@ujifgc ujifgc mentioned this pull request Oct 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants