Hidden field behavior

[ctcleary]

As requested in #93 the behavior is now:

• A user enters data into a field "X" e.g. "This is an answer"
• "X" becomes hidden.
• Save action does not persist the data in "X"
• "X" becomes visible, "This is an answer" is still populated in the now-shown field.

To do this I had to entirely rework the hiding mechanism. The per-type hiding behavior is no longer needed. Since every input is constructed through /types/children.js, the hiding behavior is now mostly centralized there.

• The field's field.show object is assigned a field.show.hidden boolean so we can determine when its hidden status has changed.
• Whenever a field's hidden status changes, we register it with the controller from content-edit/index.js since this component's data is what gets sent to the save action.
• In the save action, we create a copy of the current data and set any hidden field's data to null in the clone, this filtered data is what ends up persisted to the datastore.

[Fixes #93]

[ctcleary]

Doh. Test failures and travis is yelling about style stuff.

[tivac]

Why are the hidden fields tracked/modified in content-edit/index.js? I don't see any place where it ever actually uses that data.

[ctcleary]

The hidden fields are tracked in content-edit/index.js because that's where all other field data is tracked. Then during the content-edit/head.js : ctrl.save() operation, we call filterHidden() using the data tracked via content-edit/index.js : ctrl.registerHidden() to remove any currently-hidden field's data from the update. This is the only way (afai can tell) we can retain any information in a hidden field for the desired UX (hiding/unhiding a field does not clear it) without accidentally saving the data from a hidden field when the save button is hit.

[tivac]

Ah, sure. I need to stew on they way that's wired up for a bit because right now it's very bodged together and I forgot just how clunky it was.

I'd like the tests to be cleaned up (either removed if now-irrelevant or fixed) and then I think we're good to merge. This will need to be rebased on master before that can happen, but that should be pretty small.

[tivac]

PS I said this in-person but for posterity: per-type hiding logic was really stupid and I'm glad it's removed.

[ctcleary]

I would also like to clean it up a bit, but for now I was shooting to get the new hiding behavior wired up without a total overhaul of any other important systems. Was waiting to update the tests until I got the :+1: so I'll get on that part probably tomorrow AM.

[ctcleary]

This was repeatedly delayed because every time I tried to figure out why I couldn't write tests for types/children.js I would run into really bizarre errors I couldn't figure out and eventually had to put it aside again -- lest I totally lost my mind and bloody rage vomited at my computer like a 28 Days Later zombie.

Turns out the problem is this: Since types/children.js imports like half the project, it ends up importing some files with code that assumes the presence of window, document, and require. Since these do not (as far as I can tell) exist in Mocha's headless browser, Mocha just pukes and refuses to work when it hits them.

I tried to go throw some "just for testing" patches over these spots (var document = document || { ... }), but I think I've determined that the problem is pervasive enough that it's not a good path forward.

I'm inclined to say we skip tests on types/children.js for now so that we can get this preferred functionality in.

I'm also inclined to say we punt testing forward and make a future task for making types/children.js more testable by reducing its imports, but I also know that's not super likely to happen.

So, ball's in your court, @tivac -- What'chu'wan'me'do?

[tivac]

¯_(ツ)_/¯

Skipping tests on children.js is fine for now.

[ctcleary]

Tests removed. I think most of the style warnings precede me?

Ready for merge?