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

Ignore disabled fields #108

Merged
merged 9 commits into from
May 1, 2015
Merged

Ignore disabled fields #108

merged 9 commits into from
May 1, 2015

Conversation

wmadden
Copy link

@wmadden wmadden commented Apr 21, 2015

No description provided.

HiroAgustin and others added 3 commits March 16, 2015 11:19
`Backbone.Syphon` is referenced as `Backbone.Syhpon` multiple times.
@wmadden
Copy link
Author

wmadden commented Apr 21, 2015

@samccone yes.

@wmadden
Copy link
Author

wmadden commented Apr 21, 2015

@samccone #70 (comment)

@samccone
Copy link
Member

I should have been more clear, it is a good choice to do it in this code path.
Given where this logic lives It seems like this would be a hard thing to disable if someone wanted to for whatever reason.

What do you think @wmadden

@wmadden
Copy link
Author

wmadden commented Apr 21, 2015

Good point @samccone. I'd still opt to ignore disabled elements by default since that's the behaviour implemented by browsers. I think if someone wants to specifically include their disabled elements they can do so by a) not disabling them (greying them out in CSS for example) or b) opening their own pull request.

@wmadden
Copy link
Author

wmadden commented Apr 21, 2015

@samccone I rewrote it using @rhubarbselleven's type selection code. This is now easy for people to customize if they want to.

I'm also requesting that you update your major branch to this point, since the type selector changes were only available on master.

@wmadden
Copy link
Author

wmadden commented Apr 23, 2015

@samccone any progress here?

@samccone
Copy link
Member

Lgtm 👍 @rhubarbselleven Want to give it the once over

rhubarbselleven added a commit that referenced this pull request May 1, 2015
@rhubarbselleven rhubarbselleven merged commit d69ca02 into marionettejs:major May 1, 2015
@rhubarbselleven
Copy link
Contributor

💥

Thanks guys.

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