In #1452188: New UI for string translation we copied some parts of tabledrag warning to the string translation ui, to show the user that he changed something and needs to save this. We would also like to have some dirty forms functionalities which warns the user of leaving the page without saving changes.
The idea would be to have a general JS functionality which can be easily used by core and contrib to inform users about changes (moving the current tabledrag into more general).
Or even better to have a dirtyform (but this would be probably way harder).
There is a DirtyForms implementation for D6: http://drupal.org/project/dirtyforms
And a project application for D7: #1503176: jQuery Dirty Forms which uses the JQuery DirtyForms Plugin http://mal.co.nz/code/jquery-dirty-forms/
Comment | File | Size | Author |
---|---|---|---|
#22 | core-js-fix-form-1636992-22.patch | 1.1 KB | nod_ |
#16 | core-js-fix-form-1636992-16.patch | 4.44 KB | nod_ |
#16 | core-js-fix-form-1636992-16-bis-do-not-test.patch | 20.31 KB | nod_ |
Comments
Comment #1
mgiffordTagging
Comment #2
nod_That's a nice idea. I read the code for dirty form plugin. Let's just keep the idea, the code is not up to core standards and has too much things in it we won't be using.
Comment #3
nod_sorry about that
Comment #4
nod_There is two issue here really:
1) Detect a form has changed values (checking against default value, or stored initial values – can be tricky for FF).
2) Display an alert, stop the user from hurting himself.
And I really mean to separate the two, as shown by #1637404: Improve Drupal.behaviors.formUpdated, there is a need for contrib to access this information programatically.
Comment #5
Wim LeersTo add to #4.1: it might make sense to also support
jQuery('[content-editable]')
. There, it would need to bind toblur keyup paste
(and of course: compare with the original values to make sure something has actually changed).Comment #6
Wim LeersAnother bug report due to
formUpdated
being sucky: #2099331: Save button does not appear when using quick edit to remove an image from a post. I.e. when using thefile
render element, clicking the AJAXy "Remove" button should triggerformUpdated
, but it does not.Attempting to improve the issue title. Feel free to improve further.
Comment #7
Wim LeersThe exact same bug was reported again, and it's still
formUpdated
being crappy that's at fault.This sorely lacking feature leads to critical UI problems. Bumping to critical.
Comment #8
Bojhan CreditAttribution: Bojhan commentedI agree, this is pretty horrible. Every field should by default work within Edit module.
Comment #9
nod_May be a bit too much going on in this patch:
Fix formUpdated, should work properly and much less often than before. Can't test the remove image in edit since the whole functionality is busted. It triggers as expected when using the backend though.
The JS clean-up done on the active class issue is needed here as well to remove a couple of ugly lines.
Added as a bonus, the file validation in JS is fixed (yes, it was broken).
I don't think this works on unmanaged file inputs, but maybe it does, not sure.
Comment #10
Wim LeersHURRAY! nod_++! Thanks for working on this!
Busted in-place editing of image fields confirmed: WTF!? Issue opened: #2177289: In-place editing of image fields is broken.
Attached reroll only fixes docs/whitespace.
Comment #11
Wim LeersComment #12
nod_Only change is adding the detach function.
Comment #13
nod_This should be RTBC-able.
Comment #14
Wim LeersThe remaining todo was solved, the new todo in #13 is blocked on another issue. All is working well as per #10. In #10, I rerolled the patch, but I only changed whitespace/docs. Therefor I can RTBC :)
Comment #15
webchickWhile I appreciate the completeness of bug fixing done here, this patch is a total kitten-eating monster. :( Can we please have a patch that just solves the problem at hand?
Comment #16
nod_"minimum" version of the code is an undocumentable mess. If you look at the code after you applied the previous patch and this one, tell me which one is easier to understand. Readable patch is hardly a criteria if the resulting code is ugly.
2 patches:
- the minimum amount of changes.
- the one with code that's actually maintainable without being a PITA (to apply on top of the previous one).
Comment #17
Wim LeersManually tested again, same results as in #10.
Comment #18
nod_Will take the validation stuff and the refactor in separate issues (see child issues), please commit the 4kb patch of #16 so I can get the rest rolling.
Comment #19
catchOK I've gone ahead and committed the 4kb patch. Thanks!
Comment #20
Wim LeersComment #21
droplet CreditAttribution: droplet commentedif contextIsForm is TRUE, it skipped apply "ONCE" function. Also, it need not to find the 'form' anymore ?
$(context) = $form
Comment #22
nod_Right, so the init is skiped if context is a form indeed.
For now all the ajax requests in this case are for forms that are deleted after they are submitted so it won't trigger any bugs. But it might once contrib get in there.
Follow-up patch adds the once and removeOnce when context is form.
Comment #23
droplet CreditAttribution: droplet commentedComment #24
nod_Comment #25
webchickCommitted and pushed to 8.x. Thanks!