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

Projects
None yet
4 participants
@dariocravero
Contributor

dariocravero commented Feb 12, 2013

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

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Feb 12, 2013

Member

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.

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

This comment has been minimized.

Show comment
Hide comment
@dariocravero

dariocravero Feb 12, 2013

Contributor

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.

Contributor

dariocravero commented Feb 12, 2013

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

This comment has been minimized.

Show comment
Hide comment
@nesquena

nesquena Mar 10, 2013

Member

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

Member

nesquena commented Mar 10, 2013

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

@dariocravero

This comment has been minimized.

Show comment
Hide comment
@dariocravero

dariocravero Mar 13, 2013

Contributor

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

Contributor

dariocravero commented Mar 13, 2013

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

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Mar 13, 2013

Member

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.

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

This comment has been minimized.

Show comment
Hide comment
@DAddYE
Member

DAddYE commented Mar 20, 2013

@dariocravero status?

@dariocravero

This comment has been minimized.

Show comment
Hide comment
@dariocravero

dariocravero Mar 20, 2013

Contributor

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
.

Contributor

dariocravero commented Mar 20, 2013

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

Merge pull request #1054 from padrino/add-disabled-attribute-to-selec…
…t-options

Implemented disabled attribute for select_tag form helper

@DAddYE DAddYE merged commit c409e42 into master Jul 1, 2013

1 check passed

default The Travis build passed
Details
@DAddYE

This comment has been minimized.

Show comment
Hide comment
@DAddYE

DAddYE Jul 1, 2013

Member

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

Member

DAddYE commented Jul 1, 2013

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

@ujifgc ujifgc referenced this pull request Oct 16, 2013

Merged

Cleanup helpers #1457

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