Last Comment Bug 797845 - Disable save button if no edits have been made
: Disable save button if no edits have been made
Status: RESOLVED FIXED
:
Product: Mozilla Developer Network
Classification: Other
Component: Editing (show other bugs)
: unspecified
: All All
: -- minor (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: website
:
Mentors: Eric Shepherd [:sheppy]
Depends on: 1065965 1066048
Blocks: 957649 1018206 1032827
  Show dependency treegraph
 
Reported: 2012-10-04 07:07 PDT by Eric Shepherd [:sheppy]
Modified: 2015-04-20 06:05 PDT (History)
15 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Eric Shepherd [:sheppy] 2012-10-04 07:07:55 PDT
It was pointed out in bug 797102 that it would be nice to know if you've made any changes. Let's disable (and visibly dim) the save button(s) when no edits have been made, so the user knows they haven't done anything that needs saving.
Comment 1 David Walsh :davidwalsh 2013-04-16 14:57:16 PDT
Realize that this means more than just editing the WYSIWYG; this means all fields within the document form...
Comment 2 jhancock1229 2013-05-13 10:24:32 PDT
This is my first time contributing to anything, I would like to work on this. Any tips on how I could get into this would be great.
Comment 3 David Walsh :davidwalsh 2013-05-13 10:26:19 PDT
Thanks for wanting to help out jhancock1229!  The first step is getting set up with Kuma.  Documented instructions are here:

https://github.com/mozilla/kuma/blob/master/docs/installation-vagrant.rst

Feel free to jump into the #mdndev room on Mozilla IRC to ask questions!

Excited to have your help!
Comment 4 Ram [:websigh] 2013-09-26 19:31:26 PDT
Hey! If this feature is still welcome, I'll submit the fix soon.
Comment 5 John Karahalis [:openjck] 2013-09-27 10:09:36 PDT
Sounds great, Ram! Please see this guide on contributing.

https://github.com/mozilla/kuma/blob/master/CONTRIBUTING.md
Comment 6 Eric Shepherd [:sheppy] 2013-10-02 09:38:44 PDT
(In reply to Ram [:websigh] from comment #4)
> Hey! If this feature is still welcome, I'll submit the fix soon.


Yes please!
Comment 7 David Walsh :davidwalsh 2013-10-02 09:39:53 PDT
There's already a pull request in for this.
Comment 8 Ram [:websigh] 2013-10-03 17:50:39 PDT
Just a heads up, I made the watchedElements list much less specific than it was, focusing on only two element ids since I thought $("form") might be too general. Let me know if you just want me to use "form" instead.
Comment 9 James Bennett [:ubernostrum] 2013-10-05 16:55:29 PDT
Since the underlying issue here is finding a way to let a user know whether they have unsaved changes, have we fully explored whether disabling the save button is the best way to do so? It feels like we could easily run into issues where something *has* changed, just not something that this JavaScript is currently wired up to watch for, which is potentially bad. And I suspect there are other ways we can inform the user that they have changes waiting to be saved -- I know there are JS hooks available when they try to close the tab or navigate away, for example, to pop a "are you sure you want to do that, you've got unsaved changes" notification.
Comment 10 Eric Shepherd [:sheppy] 2013-10-06 05:59:35 PDT
(In reply to James Bennett [:ubernostrum] from comment #9)
> Since the underlying issue here is finding a way to let a user know whether
> they have unsaved changes, have we fully explored whether disabling the save
> button is the best way to do so? It feels like we could easily run into
> issues where something *has* changed, just not something that this
> JavaScript is currently wired up to watch for, which is potentially bad. And
> I suspect there are other ways we can inform the user that they have changes
> waiting to be saved -- I know there are JS hooks available when they try to
> close the tab or navigate away, for example, to pop a "are you sure you want
> to do that, you've got unsaved changes" notification.

That's a good point; maybe it would be better to have some kind of "there are unsaved changes" indicator and a warning on closing the tab.
Comment 11 Ram [:websigh] 2013-10-07 11:50:46 PDT
I'm fine with rewriting the fix to "indicator+closing warning." 

What might serve as a good indicator of change? The most obvious thing that comes to mind is placing [+] or • next to the document's title, but I'm sure there's a better way to do it.
Comment 12 Eric Shepherd [:sheppy] 2013-10-07 11:56:09 PDT
(In reply to Ram [:websigh] from comment #11)
> I'm fine with rewriting the fix to "indicator+closing warning." 
> 
> What might serve as a good indicator of change? The most obvious thing that
> comes to mind is placing [+] or • next to the document's title, but I'm sure
> there's a better way to do it.

That could work. Or maybe change the color of the save button?
Comment 13 Himani Agrawal 2014-02-20 05:40:35 PST
(In reply to Eric Shepherd [:sheppy] from comment #12)
> (In reply to Ram [:websigh] from comment #11)
> > I'm fine with rewriting the fix to "indicator+closing warning." 
> > 
> > What might serve as a good indicator of change? The most obvious thing that
> > comes to mind is placing [+] or • next to the document's title, but I'm sure
> > there's a better way to do it.
> 
> That could work. Or maybe change the color of the save button?

is this bug fixed ?
Comment 14 Eric Shepherd [:sheppy] 2014-02-20 08:02:06 PST
(In reply to himani93 from comment #13)

> is this bug fixed ?

Not that I know of...
Comment 15 David Walsh :davidwalsh 2014-07-17 14:06:15 PDT
IMO doing this on the client side is the wrong approach.  We should be analytics data received on the server side and not creating new revisions in cases where nothing has changed.  Disabling a submit button will only cause problems.
Comment 16 Justin Crawford [:hoosteeno] [:jcrawford] 2014-07-17 14:10:10 PDT
:sheppy, can you clarify the problem you want to solve with this bug? Sounds like we're bouncing around a lot of solutions that each solve a different problem.
Comment 17 Janet Swisher 2014-07-17 14:50:42 PDT
Sheppy's on PTO until next week. I'm happy as long as we stop saving empty revisions. The UI details are less important to me, though visual cues to the user are nice to have.
Comment 18 Eric Shepherd [:sheppy] 2014-07-20 15:41:59 PDT
The primary problem is the wasteful creation of null revisions to articles. There should never be a "revision" added to an article when there have been no changes.

There are many reasons why null revisions are a problem, including:

* They make it harder to understand the true modification history of a page.
* They blow up the data store unnecessarily.
* They inflate individual users' contribution levels with "junk" saves.

This is a proposed solution that also offers the UX benefit of showing the user that they've not changed anything. It can be nice to know, when looking at the page, whether or not you've made any unsaved changes.

But preventing null revisions is the primary purpose.
Comment 19 David Walsh :davidwalsh 2014-07-20 18:01:39 PDT
....and since revisions can be created via the API and with JavaScript turned off, the server side solutions is most important.  Per principle, client side is to be treated as an enhancement.  In this case, a server side solutions is much more reliable and important.
Comment 20 Eric Shepherd [:sheppy] 2014-07-20 18:58:13 PDT
(In reply to David Walsh :davidwalsh from comment #19)
> ....and since revisions can be created via the API and with JavaScript
> turned off, the server side solutions is most important.  Per principle,
> client side is to be treated as an enhancement.  In this case, a server side
> solutions is much more reliable and important.

Yes; that's why we have a two-pronged plan to handle this problem -- fix the server side to not accept null revisions, then enhance the client side to provide appropriate UX indications that no changes have been made (as described in this bug).
Comment 21 Māris Fogels [:mars] 2014-07-22 14:26:45 PDT
Also see tracker bug 1018206, which follows all of the different fixes needed to solve the empty revisions problem.
Comment 22 Ruben Vereecken 2014-08-30 09:47:40 PDT
I submitted a PR for this one. It only solves the client side, Sheppy said that would be fine. Null revisions can still be made if attempted through the API.
Comment 23 Luke Crouch [:groovecoder] 2014-09-08 09:03:52 PDT
Discussed during PR triage: https://github.com/mozilla/kuma/pull/2722
Comment 24 [github robot] 2014-09-10 12:04:48 PDT
Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/e2f0b5834bc0c2d299014eb84ba9a6cafbe8b90a
fix bug 797845 - Disable save button if no edits have been made

https://github.com/mozilla/kuma/commit/34277184c22144e13ab64856823e1b4141a65fc5
Merge pull request #2731 from darkwing/new-issue-797845

fix bug 797845 - Disable save button if no edits have been made
Comment 25 Luke Crouch [:groovecoder] 2014-09-10 13:06:31 PDT
Note: this is pushed behind a waffle flag and we've enabled it first for admin editors.
Comment 26 Luke Crouch [:groovecoder] 2014-09-10 13:10:33 PDT
Caught a bug in prod that I didn't notice locally:

STR:
Edit a page
Make no changes
Click the "Save and Keep Editing" button **in the ckeditor toolbar**

Expected result:
Nothing

Actual result:
Blue "Saving changes…" notification in the top right

Ruben, do you think you could fix up that last little bug? Or should someone else grab it?
Comment 27 Ruben Vereecken 2014-09-11 03:47:33 PDT
I'll definitely look into it. It's strange though, I never touched the saving part myself.
Comment 28 Māris Fogels [:mars] 2014-10-07 14:50:15 PDT
Has comment 26 been resolved?  Can we mark this as FIXED?
Comment 29 David Walsh :davidwalsh 2014-10-07 14:54:42 PDT
Yep, resolved long ago.
Comment 30 Luke Crouch [:groovecoder] 2015-01-14 21:47:50 PST
This has regressed with the CKE4 update. :( To reproduce:

1. Go to https://developer.mozilla.org/en-US/docs/User:codegroover$edit
2. Focus into the CKE box (I clicked with mouse)
3. Wait a few seconds and an "Insert paragraph here" element appears
4. The new element activates the save buttons long enough that you can click or use keyboard shortcut to save even though you've not made any changes.
Comment 31 Eric Shepherd [:sheppy] 2015-03-12 14:44:55 PDT
I cannot reproduce this. Indeed, I never see any "Insert paragraph here". Where does that come from and where does it appear?
Comment 32 Eric Shepherd [:sheppy] 2015-04-17 18:40:23 PDT
Luke -- are you still seeing this problem? I still can't reproduce it.
Comment 33 Luke Crouch [:groovecoder] 2015-04-20 06:05:34 PDT
Nope, I can't reproduce it anymore.

Note You need to log in before you can comment on or make changes to this bug.