Skip to content

Fix SC.Button form submission issues by adding a configurable type property#11

Merged
wycats merged 7 commits intoemberjs:masterfrom
ebryn:sc-button
Jun 18, 2011
Merged

Fix SC.Button form submission issues by adding a configurable type property#11
wycats merged 7 commits intoemberjs:masterfrom
ebryn:sc-button

Conversation

@ebryn
Copy link
Member

@ebryn ebryn commented Jun 3, 2011

No description provided.

@wycats
Copy link
Member

wycats commented Jun 3, 2011

Can you include test cases?

@ebryn
Copy link
Member Author

ebryn commented Jun 3, 2011

How do those look?

@mgood
Copy link
Contributor

mgood commented Jun 5, 2011

Ah, I'd been trying something similar and also noticed that my renderBuffer wasn't being called, so I ended up overriding applyAttributesToBuffer like this:

applyAttributesToBuffer: function(buffer) {
this._super(buffer);
var buttonType = get(this, 'buttonType');
if (buttonType) {
buffer.attr('type', buttonType);
}
},

WDYT?

@ebryn
Copy link
Member Author

ebryn commented Jun 6, 2011

I think that the Handlebars view helper should be calling into the view it's rendering to use it's renderBuffer rather than ignoring it and creating one in the view helper.

@tomdale
Copy link
Member

tomdale commented Jun 16, 2011

I'm not super comfortable with overriding the render buffer to add attributes. Maybe we should consider switching SC.Button to use a defaultTemplate, like SC.Checkbox does?

@trek
Copy link
Member

trek commented Jun 16, 2011

I had to take a similar approach for wrapping native option and needing to set the value attribute: https://gist.github.com/1027727

Using the collection helper so I could use bindAttr inserted additional markup into the option tags that the browser wouldn't happily parse.

With native form controls there's occasionally a need to set the tagName and be able to supply certain attributes on the DOM representation. I also couldn't find another way to accomplish this currently except for overriding applyAttributesToBuffer()

Maybe have an array of properties that, when set on the view, are passed to the element? Or having a similar approach to item* property names

SC.Button = SC.View.extend({
  tagAttributes: ['type'],
  ...
})

{{#view SC.Button type='submit'}}
  Click Me
{{/view}}

or


SC.Button = SC.View.extend({
  ...
})

{{#view SC.Button elemenType='submit'}}
  Click Me
{{/view}}

@ebryn
Copy link
Member Author

ebryn commented Jun 16, 2011

tomdale: The problem with using a defaultTemplate is that SC.Button uses the button tag which has it's value rendered inside of the tag, rather than as an attribute. AFAIK there isn't a way to render the content provided within the #view block in a defaultTemplate.

Greg blogged about creating a TextArea view, which has the same problem. Would you rather it be implemented like he did? http://blog.sproutcore.com/dispatches-from-the-edge-handlebars-without-spans/#more-1332

@ebryn
Copy link
Member Author

ebryn commented Jun 16, 2011

Actually, the TextArea is different. It doesn't use a #view block helper, so that makes it easier.

@tomdale
Copy link
Member

tomdale commented Jun 16, 2011

I'll add attribute/attribute binding support to make this work.

wycats added a commit that referenced this pull request Jun 18, 2011
Fix SC.Button form submission issues by adding a configurable type property
@wycats wycats merged commit 176a511 into emberjs:master Jun 18, 2011
wagenet pushed a commit to wagenet/ember.js that referenced this pull request Jul 19, 2012
…ntent during the arrayDidChange handler. This causes the modelCache in ModelArray to be out of sync. Fix was to change the order of the _super call. In general, this code seems dangerous to update the modelCache in these places.
wagenet pushed a commit to wagenet/ember.js that referenced this pull request Jul 19, 2012
bmac pushed a commit to bmac/ember.js that referenced this pull request Aug 9, 2013
ember-handlebars/lib/controls/text_support.js
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.

5 participants