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

contenteditable support #143

Merged
merged 18 commits into from
Feb 8, 2018
Merged

contenteditable support #143

merged 18 commits into from
Feb 8, 2018

Conversation

tpict
Copy link
Contributor

@tpict tpict commented Feb 8, 2018

This PR merges @davidsulc's work in #113 with the master branch. I have elected to add a new InputReader/Writer for handling contenteditable elements instead of extending the default.

rhubarbselleven and others added 15 commits March 15, 2015 12:23
…is-docker

Allows travis to use docker images for testing
…-checkboxes

An indeterminate checkbox will be a null value
Use `$.find`, not `$.children`, to parse nested inputs within a specific form
`Backbone.Syphon` is referenced as `Backbone.Syhpon` multiple times.
Form elements are now fetched with `Syphon.InputFetcher`, instead of
the private `getForm` function. By default, this will fetch `contenteditable`
elements in addition to inputs and checkboxes.

"Content editable" fields are handled as follows:

* must have `contenteditable="true"` (having only `contenteditable` present
    is *NOT* sufficient, even though it conforms to the HTML spec)
* must define a `data-name` property, which is used like a standard `input`
    field's `name` property
* data in read/written using jQuery's `html()` method
@tpict tpict changed the title contentedtiable support contenteditable support Feb 8, 2018
Copy link
Member

@scott-w scott-w left a comment

Choose a reason for hiding this comment

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

Thanks @tpict this looks great! I've got a couple of queries just to make sure you've caught all cases.

Once you've got that sorted, just bump the version to 0.8.0 and I'll ask someone to push the build to NPM.

@@ -1,6 +1,6 @@
{
"name": "backbone.syphon",
"version": "0.7.0",
Copy link
Member

Choose a reason for hiding this comment

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

Bump this to 0.8.0

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "backbone.syphon",
"version": "0.7.0",
"version": "0.7.1",
Copy link
Member

Choose a reason for hiding this comment

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

And here

@@ -149,7 +153,7 @@ var getForm = function(viewOrForm) {
if (_.isUndefined(viewOrForm.$el)) {
return $(viewOrForm).find(':input');
} else {
return viewOrForm.$(':input');
return viewOrForm.$(':input, [contenteditable]');
Copy link
Member

Choose a reason for hiding this comment

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

Have you missed off adding [contenteditable] to the _.isUndefined case above? ☝️

@tpict
Copy link
Contributor Author

tpict commented Feb 8, 2018

@scott-w done 😄

@scott-w scott-w merged commit 85f6c09 into marionettejs:master Feb 8, 2018
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.

7 participants