denverfoundation / storybase

The code behind Floodlight
http://floodlightproject.org
MIT License
11 stars 7 forks source link

Text assets should have auto-save feature #848

Closed jwirfs-brock closed 10 years ago

jwirfs-brock commented 11 years ago

This is related to issue #847.

ghing commented 11 years ago

Assigning to @jwirfs-brock to specify desired behavior.

jwirfs-brock commented 11 years ago

This is also related to issue #711 (summary save behavior is different than text asset save behavior).

jwirfs-brock commented 11 years ago

To clarify, there is a suite of issues that are related:

I think these three issues can all be solved with one solution. Either we can,

  1. Implement auto-save in text assets (the solution discussed in #847), or
  2. Add a save button to the Summary and Call to Action fields (the solution discussed in #711)

I prefer (1) to (2). @ghing do you have a preference? I'm open to discussion on this issue.

If we implement an auto-save, I have a few questions:

  1. Should we keep the "Save/Cancel" buttons, and have the auto-save be a redundancy? My initial reaction would be yes to keeping the buttons, because this keeps it consistent with other asset types.
  2. If a user types in some text (thus triggering the auto-save), then hits cancel, should that asset sill be saved (i.e., it goes into the asset drawer), or should it disappear? My initial reaction is that it should not be saved (again, to keep it consistent with other asset types).
  3. To keep the text editing consistent with the summary and text assets, should we add a "Save" button to the summary as well? I'm not sure what makes the most sense here. There is a "Save" button at the top that will save the summary (which does not save text assets that have not been individually saved), so in a way we already have this feature. Thoughts?
ghing commented 11 years ago

@jwirfs-brock:

I'm really not sure about 1 (keeping the Save/Cancel) buttons or 2 (behavior of the cancel button). I think I'll have to get a bit deeper into the implementation to decide, though I'm pretty sure some changes to the buttons (for text assets) will have to happen.

As for 3 (save button for summary), I don't think we need this, especially if we're going to reconfigure some of the metadata editing as part of the story building workflow.

ghing commented 11 years ago

Note to self:

The saving flow is submit form -> processForm() -> saveModel()

Gotchas:

Questions:

ghing commented 11 years ago

Triggering the auto-save is a little tricky.

The wysihtml5 editor has a number of events but I'm not sure which one is the best one for our case.

We could trigger autosave on:

There's some "cost" to making a request to the server every time we save our item, so we want to autosave as little as possible. Of these options, I think I like either the blur event, or the time-based event. The blur event won't save as often, but it definitely addresses the case that seems to motivate this feature: clicking away from the editor without remembering to click the save button.

I've also done a little more thinking about the buttons. I think we should keep the Cancel and Save buttons to be consistent with other asset types and the previous behavior. The behavior would be:

ghing commented 11 years ago

Brainstormed test cases:

ghing commented 11 years ago

@jwirfs-brock Reported that the text wasn't being autosaved when switching sections. I did some debugging and this is what I found:

Quirksmode has a good rundown of all the weirdness (e.g. Chrome doesn't trigger focus/blur when clicking on links with the mouse) around these events.

There doesn't seem to be a prefarable wysihtml5 event to listen to, but perhaps we could listen on a DOM event (like mouseout).

ghing commented 11 years ago

I tried listening to mouseout events like this:

        view.bodyEditor.on('focus', function() {
          var editor = this;
          $(editor.currentView.element).on('mouseout', function() {
            view.form.fields.body.editor.determineChange();
          });

          $(editor.toolbar.element).on('mousein', function() {
            view.preventDelayedSave();
          });
        })
        .on('blur', function() {
          $(this.currentView.element).off('mouseout');
          $(this.toolbar.element).off('mousein');
        });

but it didn't work great, largely due to the toolbar being outside the element itself. Next idea is to trigger autosave on keyup, but debounce the functions.