Disable save button if no edits have been made

RESOLVED FIXED

Status

Mozilla Developer Network
Editing
--
minor
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: sheppy, Unassigned, Mentored)

Tracking

(Blocks: 3 bugs)

Details

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

4 years ago
Whiteboard: [mentor=sheppy]
Realize that this means more than just editing the WYSIWYG; this means all fields within the document form...

Comment 2

4 years ago
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.
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

4 years ago
Hey! If this feature is still welcome, I'll submit the fix soon.
Sounds great, Ram! Please see this guide on contributing.

https://github.com/mozilla/kuma/blob/master/CONTRIBUTING.md
(Reporter)

Comment 6

4 years ago
(In reply to Ram [:websigh] from comment #4)
> Hey! If this feature is still welcome, I'll submit the fix soon.


Yes please!
There's already a pull request in for this.

Comment 8

4 years ago
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.
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.
(Reporter)

Comment 10

4 years ago
(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

4 years ago
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.
(Reporter)

Comment 12

4 years ago
(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?
(Reporter)

Updated

3 years ago
Blocks: 957649

Comment 13

3 years ago
(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 ?
(Reporter)

Comment 14

3 years ago
(In reply to himani93 from comment #13)

> is this bug fixed ?

Not that I know of...
Whiteboard: [mentor=sheppy] → [mentor=sheppy][good first bug]
Blocks: 997763

Updated

3 years ago
No longer blocks: 997763

Updated

3 years ago
Blocks: 1018206
(Assignee)

Updated

3 years ago
Mentor: eshepherd@mozilla.com
Whiteboard: [mentor=sheppy][good first bug] → [good first bug]
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.
: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.
Flags: needinfo?(eshepherd)

Comment 17

3 years ago
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.
(Reporter)

Comment 18

3 years ago
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.
Flags: needinfo?(eshepherd)
....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.
(Reporter)

Comment 20

3 years ago
(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).
Also see tracker bug 1018206, which follows all of the different fixes needed to solve the empty revisions problem.

Comment 22

3 years ago
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.
Discussed during PR triage: https://github.com/mozilla/kuma/pull/2722

Comment 24

3 years ago
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

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Note: this is pushed behind a waffle flag and we've enabled it first for admin editors.
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?
Status: RESOLVED → REOPENED
Flags: needinfo?(rubenvereecken)
Resolution: FIXED → ---

Comment 27

3 years ago
I'll definitely look into it. It's strange though, I never touched the saving part myself.
Flags: needinfo?(rubenvereecken)
Depends on: 1065965
Depends on: 1066048
Has comment 26 been resolved?  Can we mark this as FIXED?
Flags: needinfo?(lcrouch)
Flags: needinfo?(dwalsh)
Yep, resolved long ago.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Flags: needinfo?(dwalsh)
Resolution: --- → FIXED
Flags: needinfo?(lcrouch)
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.
Blocks: 1032827
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [good first bug]
(Reporter)

Comment 31

2 years ago
I cannot reproduce this. Indeed, I never see any "Insert paragraph here". Where does that come from and where does it appear?
Flags: needinfo?(lcrouch)
(Reporter)

Comment 32

2 years ago
Luke -- are you still seeing this problem? I still can't reproduce it.
Nope, I can't reproduce it anymore.
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago2 years ago
Flags: needinfo?(lcrouch)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.