search

codecombat / treema

jQuery plugin that generates HTML interfaces to edit JSON data defined by json-schema.
http://codecombat.github.io/treema/
MIT License
152 stars 36 forks source link

Schema select button intercepts bug #11

Closed santosh2201 closed 10 years ago

santosh2201 commented 10 years ago

I found the backspace deleting bug. It is due to using @removeSelectedNodes() twice in base.coffee after removing a node. Link to issue https://github.com/codecombat/treema/issues/6 Can you please check the patch and give feedback.

santosh2201 commented 10 years ago

Schema select button intercepts clicks on value field bug cleared. Made minor css changes in base.sass . Link to bug https://github.com/codecombat/treema/issues/12

sderickson commented 10 years ago

@santosh2201 This removes all ability to delete nodes. It's not that delete should stop deleting nodes, it's that it needs to stop deleting nodes if when you initially pressed the delete key you were editing a treema node.

Also, before I can merge any pull requests from you, you'll need to sign the CLA. Thanks!

santosh2201 commented 10 years ago

I had a conversation with @nwinter it looks like it is platform specific bug. It's working fine in my platform.

santosh2201 commented 10 years ago

@sderickson This pull request is referencing two bugs: 1)https://github.com/codecombat/treema/issues/6 2)https://github.com/codecombat/treema/issues/12 I have reverted back the code of https://github.com/santosh2201/treema/commit/42aee3912a277489658337f9629d47b6949da13f . Can you check the other bug https://github.com/codecombat/treema/issues/12 and tell whether it's fine or not?

dashed commented 10 years ago

Is it possible to create and include tests for the related resolved bugs?

sderickson commented 10 years ago

Very close! Only one thing I noticed left: if I'm on Chrome and I delete one node and hold down for a second or two, then lift and press delete one more time, the page navigates back. Need to make sure e.preventDefault() gets called in this case.

@Dashed, agreed, tests would also be good to add, to make sure this behavior isn't lost. The /test folder has lots of examples of keyboard event based tests.