Closed Bug 797845 Opened 13 years ago Closed 10 years ago

Disable save button if no edits have been made

Categories

(developer.mozilla.org Graveyard :: Editing, defect)

Product:
developer.mozilla.org Graveyard
Component:
Editing
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: sheppy, Unassigned, Mentored)

References

Details

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.
Whiteboard: [mentor=sheppy]
Realize that this means more than just editing the WYSIWYG; this means all fields within the document form...
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!
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
(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.
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.
(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.
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.
(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?
Blocks: 957649
(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 ?
(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
No longer blocks: 997763
Mentor: eshepherd
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)
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.
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.
(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.
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.
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
Status: NEW → RESOLVED
Closed: 11 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 → ---
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
Closed: 11 years ago → 11 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]
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)
Luke -- are you still seeing this problem? I still can't reproduce it.
Nope, I can't reproduce it anymore.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Flags: needinfo?(lcrouch)
Resolution: --- → FIXED
Product: developer.mozilla.org → developer.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.