Skip to content

Handle Fixnums as string fields - #21

Closed
dgalarza wants to merge 2 commits into
masterfrom
dg-fixnums
Closed

Handle Fixnums as string fields#21
dgalarza wants to merge 2 commits into
masterfrom
dg-fixnums

Conversation

@dgalarza

Copy link
Copy Markdown

No description provided.

Comment thread spec/fixtures/user_form.html Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit pick - can you change the integer classes to number?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sure, I misread the simple form class. I thought it had applied integer.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, here's the output from a simple form integer field.

<div class="input integer optional coupon_usages_allowed">
  <label class="integer optional" for="coupon_usages_allowed"></label>
  <input class="numeric integer optional" id="coupon_usages_allowed" name="coupon[usages_allowed]" step="1" type="number">
</div>

Would you like it to follow that more closely? IE: integer on the wrapping div and label, and numeric and integer on the input?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, ideally. I got lazy with the inputs and have just been copying them, but at some point I'll probably have simple_form regenerate them, and possibly add that as a dependency for testing and just build up a form properly. Maybe.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Updated.

@calebhearth

Copy link
Copy Markdown
Owner

Great looks good.

@calebhearth

Copy link
Copy Markdown
Owner

Rebased/merged. This change is now available in v0.0.6.

@calebhearth

Copy link
Copy Markdown
Owner

@derekprior You were looking for this a couple days ago as well, I believe.

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.

2 participants