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

Don't set a value attribute on label tags #2029

Merged
merged 2 commits into from May 5, 2016

Conversation

Projects
None yet
2 participants
@sshaw
Contributor

sshaw commented May 4, 2016

A value attribute on a label tag is invalid.

Not the biggest fan of the fix but since the function is already modifying the caller's hash it's the least invasive option... UPDATE just used #update instead of adding :value => nil to the caller's hash.

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc May 4, 2016

Member

Why is there :value in the first place? If anything we should fix #label to not use #default_options, I think.

Member

ujifgc commented May 4, 2016

Why is there :value in the first place? If anything we should fix #label to not use #default_options, I think.

@sshaw

This comment has been minimized.

Show comment
Hide comment
@sshaw

sshaw May 4, 2016

Contributor

Why is there :value in the first place?

default_options sets field_value on each call.

If anything we should fix #label to not use #default_options, I think.

Yeah I agree. I'll update shortly.

Contributor

sshaw commented May 4, 2016

Why is there :value in the first place?

default_options sets field_value on each call.

If anything we should fix #label to not use #default_options, I think.

Yeah I agree. I'll update shortly.

@sshaw

This comment has been minimized.

Show comment
Hide comment
@sshaw

sshaw May 5, 2016

Contributor

Actually I think doing what field_field does makes more sense: call Hash#except(:value) on default_options's return value. I think it's fine that it calls default_options, that doesn't mean that all defaults have to be used.

I've updated my commit to do that.

Contributor

sshaw commented May 5, 2016

Actually I think doing what field_field does makes more sense: call Hash#except(:value) on default_options's return value. I think it's fine that it calls default_options, that doesn't mean that all defaults have to be used.

I've updated my commit to do that.

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc May 5, 2016

Member

We can't use ActiveSupport's Hash#except.

Member

ujifgc commented May 5, 2016

We can't use ActiveSupport's Hash#except.

@sshaw

This comment has been minimized.

Show comment
Hide comment
@sshaw

sshaw May 5, 2016

Contributor

Okay, I've removed the calls to Hash#except.

Contributor

sshaw commented May 5, 2016

Okay, I've removed the calls to Hash#except.

@ujifgc ujifgc merged commit 6126b2b into padrino:0.12 May 5, 2016

@sshaw sshaw referenced this pull request May 7, 2016

Closed

Release 0.12.6 #2031

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