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

Add save button allowing user to save zone post list on demand #51

Merged
merged 7 commits into from
Apr 16, 2017

Conversation

andfinally
Copy link
Contributor

@andfinally andfinally commented Dec 8, 2016

Zoninator is so useful! We rely on it a lot. Unfortunately some users hammer it like it's a desktop app: they throw a lot of posts into a zone - sometimes 20 or more - and make a whole series of changes very rapidly, adding new posts, deleting and reordering in quick succession. In this situation, Zoninator's autosave can often fail to keep up with the changes.

We mostly see this issue when someone adds a post to the zone, immediately repositions it and leaves the admin page, assuming the post will be saved in the position they dragged it to. But Zoninator hasn't had time to register the second change, and their post will be displayed in the wrong position, or may even not exist in the zone at all.

To improve this situation I've added a "Save zone posts" button above the posts list which updates the internal post list in zoninator.currentPostOrder and triggers the reorder_posts Ajax call, saving the post list to the database.

To make the autosave activity more obvious, when Zoninator initiates an Ajax request to reorder posts I'm displaying a notification to the user, and a confirmation of successful save with a timestamp - otherwise an error notification.

When I first started using Zoninator it took me a while to realise the original "Save zone" button didn't save the zone post list, but only the zone name and description. I've changed the text of this button and made it visually less prominent. The new text hopefully makes it clear what it actually does.

(I've also tidied the CSS formatting a bit and added some comments to the JS.)

save-zone-posts

…he zone on demand. Added zoninator.forceSavePosts method to update internal post list and save the current UI post list to the zone.
…atus from ajaxSuccessCallback and any error from ajaxErrorCallback.
@mjangda
Copy link
Member

mjangda commented Dec 8, 2016

Neat! I was always pretty hesitant about moving away from the instant-save, but you I've since changed my mind (plus, you have some good arguments here) :)

Small UX thing: when saving, we should disable the button and update its text to "Saving..." instead of the separate label. The separate label is useful for the success/error messages though.

Will try to look over the code next week.

@bcampeau
Copy link
Contributor

bcampeau commented Dec 8, 2016

I actually built this once too, so I'll have to compare to my code as well. I can't for the life of me recall the project but I'll dig it up.

@andfinally
Copy link
Contributor Author

Great! Thanks Mo, I'll have a look at that button, and @bcampeau it would be interesting to see how you did it.

@andfinally
Copy link
Contributor Author

Hi guys, did anyone get a chance to look at this? We've been using this feature for a few months, and it's still proving useful.

@mjangda
Copy link
Member

mjangda commented Mar 27, 2017

Sorry for the late follow-up here, @andfinally. Just tested and it the new notifications are great and the button works well.

Should autosave continue to work with the button in place? Or should we get rid of autosave and always require a press of the save button (maybe with a clear indicator that changes are pending)? If the latter, I guess it might make sense to ease in to it (introduce the button, then in a later update get rid of autosave), since it would be quite a drastic change of behaviour.

In either case, I'm happy to merge as-is.

@andfinally
Copy link
Contributor Author

Thanks Mo! Personally I'd keep autosave - there are benefits to the belt and braces approach: autosave will help the occasional person who forgets to save manually. Our users have been working with button + autosave for months now, and everybody seems happy.

@mjangda mjangda merged commit 5c6d64a into Automattic:master Apr 16, 2017
@andfinally andfinally deleted the save-button branch April 18, 2017 09:03
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.

3 participants