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

Support disabled attribute #13

Open
jzaefferer opened this issue Sep 1, 2015 · 7 comments
Open

Support disabled attribute #13

jzaefferer opened this issue Sep 1, 2015 · 7 comments

Comments

@jzaefferer
Copy link

It should be possible to make a checkbox disabled. Is there a reason for this one to not include that? Or just missing?

@jzaefferer jzaefferer changed the title Support readonly attribute Support disabled attribute Sep 1, 2015
@jzaefferer jzaefferer reopened this Sep 1, 2015
@jzaefferer
Copy link
Author

Reopened and renamed. Disabled attribute is supported, but not implemented by this view.

@pgilad
Copy link
Member

pgilad commented Sep 1, 2015

@doratias What do you think?

@wraithgar
Copy link
Contributor

What api are we thinking then? .enable() and .disable()? Seems like a very sensible thing to support, in any case.

@cdaringe
Copy link
Member

cdaringe commented Sep 1, 2015

hi @jzaefferer, all view's accept this, and do so through the bindings object. i made a fiddle to demonstrate: http://requirebin.com/?gist=f6027f76d0d84d47a1ca

there have been a lot of discussions about which view modules should accept which DOM attrs/props. As a general rule, we've been somewhat defaulting to saying 'use the bindings' vs. add additional code for single attributes.

HTH

@cdaringe cdaringe closed this as completed Sep 1, 2015
@cdaringe cdaringe reopened this Sep 1, 2015
@jzaefferer
Copy link
Author

Well, having to extend the view isn't exactly "accepting" (or even supporting). Considering all the existing supported properties, excluding disabled seems pretty arbitrary.

If that's what its going to be, I suggest to at least improve the documentation: https://github.com/AmpersandJS/ampersand-checkbox-view#properties (pointing to source code in place of proper docs isn't that great either).

@cdaringe
Copy link
Member

cdaringe commented Sep 1, 2015

@jzaefferer, i agree it's verbose. it is certainly 'supported', however. :) definitely accepting PRs to update as you think would increase understanding and clarity.

@jzaefferer
Copy link
Author

Sorry, I won't be able to help with a PR.

dhritzkiv added a commit that referenced this issue May 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants