Added confirmation for discarding changes made to a form.

asl_articles/tests/test_articles.py

@@ -9,8 +9,9 @@ import base64
 from asl_articles.search import SEARCH_ALL_ARTICLES
 from asl_articles.tests.utils import init_tests, select_main_menu_option, select_sr_menu_option, \
     do_search, get_search_results, find_search_result, get_search_result_names, check_search_result, \
+    do_test_confirm_discard_changes, find_parent_by_class, \
     wait_for, wait_for_elem, wait_for_not_elem, find_child, find_children, \
-    set_elem_text, set_toast_marker, check_toast, send_upload_data, change_image, get_article_row, \
+    set_elem_text, set_toast_marker, check_toast, send_upload_data, change_image, remove_image, get_article_row, \
     check_ask_dialog, check_error_msg, check_constraint_warnings
 from asl_articles.tests.react_select import ReactSelect
 
@@ -101,7 +102,7 @@ def test_constraints( webdriver, flask_app, dbconn ):
     """Test constraint validation."""
 
     # initialize
-    init_tests( webdriver, flask_app, dbconn, enable_constraints=1, fixtures="publications.json" )
+    init_tests( webdriver, flask_app, dbconn, disable_constraints=False, fixtures="publications.json" )
 
     # try to create an article with no title
     dlg = create_article( {}, expected_error="Please give it a title." )
@@ -172,6 +173,40 @@ def test_constraints( webdriver, flask_app, dbconn ):
 
 # ---------------------------------------------------------------------
 
+def test_confirm_discard_changes( webdriver, flask_app, dbconn ):
+    """Test confirmation of discarding changes made to a dialog."""
+
+    # initialize
+    init_tests( webdriver, flask_app, dbconn, disable_confirm_discard_changes=False, fixtures="publications.json" )
+
+    # do the test
+    def update_react_select( elem, val ):
+        select = ReactSelect( find_parent_by_class( elem, "react-select" ) )
+        select.select_by_name( val )
+    def update_multiselect( elem, vals ):
+        select = ReactSelect( find_parent_by_class( elem, "react-select" ) )
+        select.update_multiselect_values( *vals )
+    do_test_confirm_discard_changes( "new-article", {
+        "publication": (
+            lambda elem: update_react_select( elem, "MMP News" ),
+            lambda elem: update_react_select( elem, "(none)" )
+        ),
+        "authors": (
+            lambda elem: update_multiselect( elem, ["+Joe Blow"] ),
+            lambda elem: update_multiselect( elem, ["-Joe Blow"] ),
+        ),
+        "scenarios": (
+            lambda elem: update_multiselect( elem, ["+Hill 621 [E]"] ),
+            lambda elem: update_multiselect( elem, ["-Hill 621 [E]"] ),
+        ),
+        "tags": (
+            lambda elem: update_multiselect( elem, ["+foo"] ),
+            lambda elem: update_multiselect( elem, ["-foo"] ),
+        )
+    } )
+
+# ---------------------------------------------------------------------
+
 def test_delete_article( webdriver, flask_app, dbconn ):
     """Test deleting articles."""
 
@@ -518,11 +553,9 @@ def _update_values( dlg, vals ):
     for key,val in vals.items():
         if key == "image":
             if val:
-                data = base64.b64encode( open( val, "rb" ).read() )
-                data = "{}|{}".format( os.path.split(val)[1], data.decode("ascii") )
-                change_image( find_child( ".row.image img.image", dlg ), data )
+                change_image( dlg, val )
             else:
-                find_child( ".row.image .remove-image", dlg ).click()
+                remove_image( dlg )
         elif key == "publication":
             select = ReactSelect( find_child( ".row.publication .react-select", dlg ) )
             select.select_by_name( val )

asl_articles/tests/test_publications.py

@@ -14,9 +14,10 @@ from asl_articles.search import SEARCH_ALL, SEARCH_ALL_PUBLICATIONS, SEARCH_ALL_
 from asl_articles.tests.test_articles import create_article, edit_article
 from asl_articles.tests.utils import init_tests, load_fixtures, select_main_menu_option, select_sr_menu_option, \
     do_search, get_search_results, get_search_result_names, check_search_result, \
+    do_test_confirm_discard_changes, find_parent_by_class, \
     wait_for, wait_for_elem, wait_for_not_elem, find_child, find_children, find_search_result, set_elem_text, \
     set_toast_marker, check_toast, send_upload_data, check_ask_dialog, check_error_msg, check_constraint_warnings, \
-    change_image, get_publication_row
+    change_image, remove_image, get_publication_row
 from asl_articles.tests.react_select import ReactSelect
 
 # ---------------------------------------------------------------------
@@ -101,7 +102,7 @@ def test_constraints( webdriver, flask_app, dbconn ):
     """Test constraint validation."""
 
     # initialize
-    init_tests( webdriver, flask_app, dbconn, enable_constraints=1, fixtures="publications.json" )
+    init_tests( webdriver, flask_app, dbconn, disable_constraints=False, fixtures="publications.json" )
 
     # try to create a publication with no name
     dlg = create_publication( {}, expected_error="Please give it a name." )
@@ -150,6 +151,32 @@ def test_constraints( webdriver, flask_app, dbconn ):
 
 # ---------------------------------------------------------------------
 
+def test_confirm_discard_changes( webdriver, flask_app, dbconn ):
+    """Test confirmation of discarding changes made to a dialog."""
+
+    # initialize
+    init_tests( webdriver, flask_app, dbconn, disable_confirm_discard_changes=False, fixtures="publications.json" )
+
+    # do the test
+    def update_react_select( elem, val ):
+        select = ReactSelect( find_parent_by_class( elem, "react-select" ) )
+        select.select_by_name( val )
+    def update_multiselect( elem, vals ):
+        select = ReactSelect( find_parent_by_class( elem, "react-select" ) )
+        select.update_multiselect_values( *vals )
+    do_test_confirm_discard_changes( "new-publication", {
+        "publisher": (
+            lambda elem: update_react_select( elem, "Avalon Hill" ),
+            lambda elem: update_react_select( elem, "(none)" )
+        ),
+        "tags": (
+            lambda elem: update_multiselect( elem, ["+foo"] ),
+            lambda elem: update_multiselect( elem, ["-foo"] ),
+        )
+    } )
+
+# ---------------------------------------------------------------------
+
 def test_delete_publication( webdriver, flask_app, dbconn ):
     """Test deleting publications."""
 
@@ -632,11 +659,9 @@ def _update_values( dlg, vals ):
     for key,val in vals.items():
         if key == "image":
             if val:
-                data = base64.b64encode( open( val, "rb" ).read() )
-                data = "{}|{}".format( os.path.split(val)[1], data.decode("ascii") )
-                change_image( find_child( ".row.image img.image", dlg ), data )
+                change_image( dlg, val )
             else:
-                find_child( ".row.image .remove-image", dlg ).click()
+                remove_image( dlg )
         elif key == "name":
             elem = find_child( ".row.name .react-select input", dlg )
             set_elem_text( elem, val )

asl_articles/tests/test_publishers.py

@@ -10,8 +10,9 @@ from selenium.common.exceptions import StaleElementReferenceException
 from asl_articles.search import SEARCH_ALL, SEARCH_ALL_PUBLISHERS
 from asl_articles.tests.utils import init_tests, load_fixtures, select_main_menu_option, select_sr_menu_option, \
     do_search, get_search_results, get_search_result_names, check_search_result, \
+    do_test_confirm_discard_changes, \
     wait_for, wait_for_elem, wait_for_not_elem, find_child, find_search_result, set_elem_text, \
-    set_toast_marker, check_toast, send_upload_data, change_image, get_publisher_row, \
+    set_toast_marker, check_toast, send_upload_data, change_image, remove_image, get_publisher_row, \
     check_ask_dialog, check_error_msg
 
 # ---------------------------------------------------------------------
@@ -81,7 +82,7 @@ def test_constraints( webdriver, flask_app, dbconn ):
     """Test constraint validation."""
 
     # initialize
-    init_tests( webdriver, flask_app, dbconn, enable_constraints=1, fixtures="publishers.json" )
+    init_tests( webdriver, flask_app, dbconn, disable_constraints=False, fixtures="publishers.json" )
 
     # try to create a publisher with no title
     dlg = create_publisher( {}, expected_error="Please give them a name." )
@@ -374,6 +375,17 @@ def test_clean_html( webdriver, flask_app, dbconn ):
 
 # ---------------------------------------------------------------------
 
+def test_confirm_discard_changes( webdriver, flask_app, dbconn ):
+    """Test confirmation of discarding changes made to a dialog."""
+
+    # initialize
+    init_tests( webdriver, flask_app, dbconn, disable_confirm_discard_changes=False )
+
+    # do the test
+    do_test_confirm_discard_changes( "new-publisher" )
+
+# ---------------------------------------------------------------------
+
 def test_timestamps( webdriver, flask_app, dbconn ):
     """Test setting of timestamps."""
 
@@ -462,11 +474,9 @@ def _update_values( dlg, vals ):
     for key,val in vals.items():
         if key == "image":
             if val:
-                data = base64.b64encode( open( val, "rb" ).read() )
-                data = "{}|{}".format( os.path.split(val)[1], data.decode("ascii") )
-                change_image( find_child( ".row.image img.image", dlg ), data )
+                change_image( dlg, val )
             else:
-                find_child( ".row.image .remove-image", dlg ).click()
+                remove_image( dlg )
         else:
             sel = ".row.{} {}".format( key , "textarea" if key == "description" else "input" )
             set_elem_text( find_child( sel, dlg ), val )

asl_articles/tests/utils.py

@@ -2,7 +2,9 @@
 
 import os
 import json
+import itertools
 import uuid
+import base64
 import logging
 
 import sqlalchemy
@@ -49,8 +51,10 @@ def init_tests( webdriver, flask_app, dbconn, **kwargs ):
 
     # load the home page
     if webdriver:
-        if not to_bool( kwargs.pop( "enable_constraints", False ) ):
+        if to_bool( kwargs.pop( "disable_constraints", True ) ):
             kwargs[ "disable_constraints" ] = 1
+        if to_bool( kwargs.pop( "disable_confirm_discard_changes", True ) ):
+            kwargs[ "disable_confirm_discard_changes" ] = 1
         webdriver.get( webdriver.make_url( "/", **kwargs ) )
         wait_for_elem( 2, "#search-form" )
 
@@ -160,6 +164,123 @@ def check_search_result( sr, check, expected ):
 
 # ---------------------------------------------------------------------
 
+def do_test_confirm_discard_changes( menu_id, update_fields=None ): #pylint: disable=too-many-statements
+    """Test confirmation of discarding changes made to a dialog."""
+
+    # initialize
+    image_fname = os.path.join( os.path.split(__file__)[0], "fixtures/images/1.gif" )
+
+    def get_input_fields( dlg ):
+        input_fields = itertools.chain(
+            find_children( "input", dlg ),
+            find_children( "textarea", dlg )
+        )
+        input_fields = { get_field_id(f): f for f in input_fields if f.is_displayed() }
+        # NOTE: Publishers, publications and articles all have an image, but requires special handling.
+        input_fields[ "image" ] = None
+        return input_fields
+    def get_field_id( elem ):
+        if elem.get_attribute( "class" ) == "edition":
+            # FUDGE! The publication dialog has a row with two fields ("name" and "edition").
+            # We return the "edition" field, the "name" field is handled as a ReactSelect.
+            return "edition"
+        if elem.get_attribute( "class" ) == "pageno":
+            # FUDGE! The article dialog has a row with two fields ("publication" and "pageno").
+            # We return the "pageno" field, the "publication" field is handled as a ReactSelect.
+            return "pageno"
+        elem = find_parent_by_class( elem, "row" )
+        classes = set( elem.get_attribute( "class" ).split() )
+        classes.remove( "row" )
+        assert len(classes) == 1
+        return classes.pop()
+
+    # locate all the input fields
+    select_main_menu_option( menu_id )
+    dlg = wait_for_elem( 2, ".MuiDialog-root" )
+    field_ids = get_input_fields( dlg ).keys()
+    find_child( ".cancel", dlg ).click()
+
+    def update_field( field_id, dlg, elem, setVal, val=None ):
+        # check if we're updating the image
+        if field_id == "image":
+            if setVal:
+                change_image( dlg, image_fname )
+            else:
+                remove_image( dlg )
+            return None
+        # check if a custom update function has been provided
+        if update_fields and field_id in update_fields:
+            update_fields[ field_id ][ 0 if setVal else 1 ]( elem )
+            return None
+        # update the field as text
+        prev_val = elem.get_attribute( "value" )
+        if val is None:
+            val = "TEST: {}".format( field_id ) if setVal else ""
+        set_elem_text( elem, val )
+        elem.send_keys( Keys.RETURN ) # nb: in case we have a ReactSelect
+        return prev_val
+
+    def do_test( open_dialog, setVals ):
+
+        # test each input field
+        for field_id in field_ids:
+
+            # NOTE: We can't unset a publication's name once it's been set, so there's no point continuing.
+            if menu_id == "new-publication" and field_id == "name" and not setVals:
+                continue
+
+            # open the form dialog
+            open_dialog()
+            dlg = wait_for_elem( 2, ".MuiDialog-root" )
+            input_fields = get_input_fields( dlg )
+
+            # change the next input field
+            prev_val = update_field( field_id, dlg, input_fields[field_id], setVals )
+
+            # try to cancel the dialog (should get a confirmation dialog)
+            find_child( ".cancel", dlg ).click()
+            ask = wait_for_elem( 2, "#ask" )
+            assert "Do you want to discard your changes?" in find_child( ".MuiDialogContent-root", ask ).text
+            find_child( ".cancel", ask ).click()
+
+            # NOTE: We can't unset a publication's name once it's been set, so there's no point continuing.
+            if menu_id == "new-publication" and field_id == "name":
+                find_child( ".cancel", dlg ).click()
+                ask = wait_for_elem( 2, "#ask" )
+                find_child( ".ok", ask ).click()
+                continue
+            # NOTE: Changing the image will always trigger a confirmation dialog, so there's no point continuing.
+            if field_id == "image" and not setVals:
+                find_child( ".cancel", dlg ).click()
+                ask = wait_for_elem( 2, "#ask" )
+                find_child( ".ok", ask ).click()
+                continue
+
+            # restore the original value
+            if isinstance( prev_val, str ):
+                prev_val = "   {}   ".format( prev_val )
+            update_field( field_id, dlg, input_fields[field_id], not setVals, prev_val )
+
+            # try to cancel the dialog (should work without confirmation)
+            find_child( ".cancel", dlg ).click()
+            ask = wait_for_not_elem( 2, ".MuiDialog-root" )
+
+    # test using a blank object
+    do_test( lambda: select_main_menu_option( menu_id ), True )
+
+    # test using an object with every field filled in
+    select_main_menu_option( menu_id )
+    dlg = wait_for_elem( 2, ".MuiDialog-root" )
+    input_fields = get_input_fields( dlg )
+    for field_id in input_fields:
+        update_field( field_id, dlg, input_fields[field_id], True )
+    find_child( ".ok", dlg ).click()
+    results = wait_for( 2, get_search_results )
+    assert len(results) == 1
+    do_test( lambda: select_sr_menu_option( results[0], "edit" ), False )
+
+# ---------------------------------------------------------------------
+
 def wait_for( timeout, func ):
     """Wait for a condition to become true."""
     return WebDriverWait( _webdriver, timeout, 0.1 ).until(
@@ -195,6 +316,16 @@ def find_children( sel, parent=None ):
     except NoSuchElementException:
         return None
 
+def find_parent_by_class( elem, class_name ):
+    """Find a parent element with the specified class."""
+    while True:
+        elem = elem.find_element_by_xpath( ".." )
+        if not elem:
+            return None
+        classes = set( elem.get_attribute( "class" ).split() )
+        if class_name in classes:
+            return elem
+
 # ---------------------------------------------------------------------
 
 def get_stored_msg( msg_id ):
@@ -338,15 +469,23 @@ def get_article_row( dbconn, article_id, fields ):
 
 # ---------------------------------------------------------------------
 
-def change_image( elem, image_data ):
+def change_image( dlg, fname ):
     """Click on an image to change it."""
     # NOTE: This is a bit tricky since we started overlaying the image with the "remove image" icon :-/
-    send_upload_data( image_data,
+    data = base64.b64encode( open( fname, "rb" ).read() )
+    data = "{}|{}".format( os.path.split(fname)[1], data.decode("ascii") )
+    elem = find_child( ".row.image img.image", dlg )
+    _webdriver.execute_script( "arguments[0].scrollTo( 0, 0 )", find_child( ".MuiDialogContent-root", dlg ) )
+    send_upload_data( data,
         lambda: ActionChains( _webdriver ) \
                 .move_to_element_with_offset( elem, 1, 1 ) \
                 .click().perform()
     )
 
+def remove_image( dlg ):
+    """Remove an image."""
+    find_child( ".row.image .remove-image", dlg ).click()
+
 def set_elem_text( elem, val ):
     """Set the text for an element."""
     elem.clear()

web/package-lock.json

@@ -8468,6 +8468,11 @@
       "resolved": "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz",
       "integrity": "sha1-4j8/nE+Pvd6HJSnBBxhXoIblzO8="
     },
+    "lodash.isequal": {
+      "version": "4.5.0",
+      "resolved": "https://registry.npmjs.org/lodash.isequal/-/lodash.isequal-4.5.0.tgz",
+      "integrity": "sha1-QVxEePK8wwEgwizhDtMib30+GOA="
+    },
     "lodash.memoize": {
       "version": "4.1.2",
       "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-4.1.2.tgz",

web/package.json

@@ -9,6 +9,7 @@
     "http-proxy-middleware": "^0.20.0",
     "lodash.clone": "^4.5.0",
     "lodash.clonedeep": "^4.5.0",
+    "lodash.isequal": "^4.5.0",
     "react": "^16.11.0",
     "react-dom": "^16.11.0",
     "react-drag-listview": "^0.1.6",

web/src/App.js

@@ -39,6 +39,7 @@ export default class App extends React.Component
         this._storeMsgs = this.isTestMode() && this.args.store_msgs ;
         this._disableSearchResultHighlighting = this.isTestMode() && this.args.no_sr_hilite ;
         this._disableConstraints = this.isTestMode() && this.args.disable_constraints ;
+        this._disableConfirmDiscardChanges = this.isTestMode() && this.args.disable_confirm_discard_changes ;
         this._fakeUploads = this.isTestMode() && this.args.fake_uploads ;
 
         // initialize
@@ -96,14 +97,12 @@ export default class App extends React.Component
         return ( <div> {content}
             { this.state.modalForm !== null &&
                 <ModalForm show={true} formId={this.state.modalForm.formId}
-                    title = {this.state.modalForm.title} titleColor = {this.state.modalForm.titleColor}
-                    content = {this.state.modalForm.content}
-                    buttons = {this.state.modalForm.buttons}
+                    {...this.state.modalForm}
                     ref = {this._modalFormRef}
                 />
             }
             { this.state.askDialog !== null &&
-                <AskDialog show={true} content={this.state.askDialog.content} buttons={this.state.askDialog.buttons} />
+                <AskDialog show={true} {...this.state.askDialog} />
             }
             <ToastContainer position="bottom-right" hideProgressBar={true} />
             { this._storeMsgs && <div>
@@ -354,6 +353,7 @@ export default class App extends React.Component
 
     isTestMode() { return process.env.REACT_APP_TEST_MODE ; }
     isDisableConstraints() { return this._disableConstraints ; }
+    isDisableConfirmDiscardChanges() { return this._disableConfirmDiscardChanges ; }
     isFakeUploads() { return this._fakeUploads ; }
     setTestAttribute( obj, attrName, attrVal ) {
         // set an attribute on an element (for testing porpoises)

web/src/ArticleSearchResult2.js

@@ -5,7 +5,7 @@ import { NEW_ARTICLE_PUB_PRIORITY_CUTOFF } from "./constants.js" ;
 import { PublicationSearchResult } from "./PublicationSearchResult.js" ;
 import { gAppRef } from "./index.js" ;
 import { ImageFileUploader } from "./FileUploader.js" ;
-import { makeScenarioDisplayName, parseScenarioDisplayName, checkConstraints, sortSelectableOptions, unloadCreatableSelect, isNumeric } from "./utils.js" ;
+import { makeScenarioDisplayName, parseScenarioDisplayName, checkConstraints, confirmDiscardChanges, sortSelectableOptions, unloadCreatableSelect, isNumeric } from "./utils.js" ;
 
 // --------------------------------------------------------------------
 
@@ -18,16 +18,25 @@ export class ArticleSearchResult2
         let refs = {} ;
         const isNew = Object.keys( vals ).length === 0 ;
 
+        // prepare to save the initial values
+        let initialVals = null ;
+        function onReady() {
+            if ( ! initialVals )
+                initialVals = unloadVals() ;
+        }
+
         // initialize the image
         let imageFilename=null, imageData=null ;
         let imageRef=null, uploadImageRef=null, removeImageRef=null ;
         let imageUrl = gAppRef.makeFlaskUrl( "/images/article/" + vals.article_id ) ;
         imageUrl += "?foo=" + Math.random() ; // FUDGE! To bypass the cache :-/
-        let onMissingImage = (evt) => {
+        function onImageLoaded() { onReady() ; }
+        function onMissingImage() {
             imageRef.src = "/images/placeholder.png" ;
             removeImageRef.style.display = "none" ;
+            onReady() ;
         } ;
-        let onUploadImage = (evt) => {
+        function onUploadImage( evt ) {
             if ( evt === null && !gAppRef.isFakeUploads() ) {
                 // nb: the article image was clicked - trigger an upload request
                 uploadImageRef.click() ;
@@ -39,7 +48,7 @@ export class ArticleSearchResult2
                 imageData = data ;
             } ) ;
         } ;
-        let onRemoveImage = () => {
+        function onRemoveImage() {
             imageData = "{remove}" ;
             imageRef.src = "/images/placeholder.png" ;
             removeImageRef.style.display = "none" ;
@@ -110,9 +119,22 @@ export class ArticleSearchResult2
         const content = <div>
             <div className="image-container">
                 <div className="row image">
-                    <img src={imageUrl} className="image" onError={onMissingImage} onClick={() => onUploadImage(null)} ref={r => imageRef=r} alt="Upload image." title="Click to upload an image for this article." />
-                    <img src="/images/delete.png" className="remove-image" onClick={onRemoveImage} ref={r => removeImageRef=r} alt="Remove image." title="Remove the article's image." />
-                    <input type="file" accept="image/*" onChange={onUploadImage} style={{display:"none"}} ref={r => uploadImageRef=r} />
+                    <img src={imageUrl} className="image"
+                        onLoad = {onImageLoaded}
+                        onError = {onMissingImage}
+                        onClick = { () => onUploadImage(null) }
+                        ref = { r => imageRef=r }
+                        alt="Upload image." title="Click to upload an image for this article."
+                    />
+                    <img src="/images/delete.png" className="remove-image"
+                        onClick = {onRemoveImage}
+                        ref = { r => removeImageRef=r }
+                        alt="Remove image." title="Remove the article's image."
+                    />
+                    <input type="file" accept="image/*" style={{display:"none"}}
+                        onChange = {onUploadImage}
+                        ref = { r => uploadImageRef=r }
+                    />
                 </div>
             </div>
             <div className="row title"> <label className="top"> Title: </label>
@@ -154,43 +176,48 @@ export class ArticleSearchResult2
             </div>
         </div> ;
 
+        function unloadVals() {
+            let newVals = {} ;
+            for ( let r in refs ) {
+                if ( r === "pub_id" )
+                    newVals[ r ] = refs[r].state.value && refs[r].state.value.value ;
+                else if ( r === "article_authors" ) {
+                    let vals = unloadCreatableSelect( refs[r] ) ;
+                    newVals.article_authors = [] ;
+                    vals.forEach( v => {
+                        if ( v.__isNew__ )
+                            newVals.article_authors.push( v.label ) ; // nb: string = new author name
+                        else
+                            newVals.article_authors.push( v.value ) ; // nb: integer = existing author ID
+                    } ) ;
+                } else if ( r === "article_scenarios" ) {
+                    let vals =  unloadCreatableSelect( refs[r] ) ;
+                    newVals.article_scenarios = [] ;
+                    vals.forEach( v => {
+                        if ( v.__isNew__ )
+                            newVals.article_scenarios.push( parseScenarioDisplayName( v.label ) ) ; // nb: array = new scenario
+                        else
+                            newVals.article_scenarios.push( v.value ) ; // nb: integer = existing scenario ID
+                    } ) ;
+                } else if ( r === "article_tags" ) {
+                    let vals =  unloadCreatableSelect( refs[r] ) ;
+                    newVals[ r ] =  vals.map( v => v.label ) ;
+                } else
+                    newVals[ r ] = refs[r].value.trim() ;
+            }
+            newVals._hasImage = ( removeImageRef.style.display !== "none" ) ;
+            return newVals ;
+        }
+
         // prepare the form buttons
         const buttons = {
             OK: () => {
-                // unload the new values
-                let newVals = {} ;
-                for ( let r in refs ) {
-                    if ( r === "pub_id" )
-                        newVals[ r ] = refs[r].state.value && refs[r].state.value.value ;
-                    else if ( r === "article_authors" ) {
-                        let vals = unloadCreatableSelect( refs[r] ) ;
-                        newVals.article_authors = [] ;
-                        vals.forEach( v => {
-                            if ( v.__isNew__ )
-                                newVals.article_authors.push( v.label ) ; // nb: string = new author name
-                            else
-                                newVals.article_authors.push( v.value ) ; // nb: integer = existing author ID
-                        } ) ;
-                    } else if ( r === "article_scenarios" ) {
-                        let vals =  unloadCreatableSelect( refs[r] ) ;
-                        newVals.article_scenarios = [] ;
-                        vals.forEach( v => {
-                            if ( v.__isNew__ )
-                                newVals.article_scenarios.push( parseScenarioDisplayName( v.label ) ) ; // nb: array = new scenario
-                            else
-                                newVals.article_scenarios.push( v.value ) ; // nb: integer = existing scenario ID
-                        } ) ;
-                    } else if ( r === "article_tags" ) {
-                        let vals =  unloadCreatableSelect( refs[r] ) ;
-                        newVals[ r ] =  vals.map( v => v.label ) ;
-                    } else
-                        newVals[ r ] = refs[r].value.trim() ;
-                }
+                // unload and validate the new values
+                let newVals = unloadVals() ;
                 if ( imageData ) {
                     newVals.imageData = imageData ;
                     newVals.imageFilename = imageFilename ;
                 }
-                // check the new values
                 const required = [
                     [ () => newVals.article_title === "", "Please give it a title.", refs.article_title ],
                 ] ;
@@ -209,11 +236,24 @@ export class ArticleSearchResult2
                     () => notify( newVals, refs )
                 ) ;
             },
-            Cancel: () => { gAppRef.closeModalForm() ; },
+            Cancel: () => {
+                let newVals = unloadVals() ;
+                if ( initialVals._hasImage && newVals._hasImage && imageData ) {
+                    // FUDGE! The image was changed, but we have no way to tell if it's the same image or not,
+                    // so we play it safe and force a confirmation.
+                    newVals._justDoIt = true ;
+                }
+                confirmDiscardChanges( initialVals, newVals,
+                    () => { gAppRef.closeModalForm() }
+                ) ;
+            },
         } ;
 
         // show the form
-        gAppRef.showModalForm( "article-form", isNew?"New article":"Edit article", "#d3edfc", content, buttons ) ;
+        gAppRef.showModalForm( "article-form",
+            isNew ? "New article" : "Edit article", "#d3edfc",
+            content, buttons
+        ) ;
     }
 
 }

web/src/ModalForm.js

@@ -13,6 +13,8 @@ export default class ModalForm extends React.Component
 {
 
     render() {
+
+        // initialize the buttons
         let buttons = [] ;
         for ( let btn in this.props.buttons ) {
             buttons.push(
@@ -21,7 +23,11 @@ export default class ModalForm extends React.Component
                 > {btn} </Button>
             ) ;
         }
-        return ( <Dialog id={this.props.formId} className="modal-form" open={true} onClose={this.onClose.bind(this)} disableBackdropClick>
+
+        // show the dialog
+        return ( <Dialog id={this.props.formId} className="modal-form" open={true} disableBackdropClick
+                   onClose = { this.onClose.bind( this ) }
+                 >
             <DialogTitle style={{background:this.props.titleColor}}> {this.props.title} </DialogTitle>
             <DialogContent dividers>
                 { typeof this.props.content === "function" ? this.props.content() : this.props.content }

web/src/PublicationSearchResult2.js

@@ -4,7 +4,7 @@ import CreatableSelect from "react-select/creatable" ;
 import ReactDragListView from "react-drag-listview/lib/index.js" ;
 import { gAppRef } from "./index.js" ;
 import { ImageFileUploader } from "./FileUploader.js" ;
-import { checkConstraints, sortSelectableOptions, unloadCreatableSelect, ciCompare, isNumeric } from "./utils.js" ;
+import { checkConstraints, confirmDiscardChanges, sortSelectableOptions, unloadCreatableSelect, ciCompare, isNumeric } from "./utils.js" ;
 
 // --------------------------------------------------------------------
 
@@ -16,18 +16,34 @@ export class PublicationSearchResult2
         // initialize
         let refs = {} ;
         let imageFilename=null, imageData=null ;
+        let imageRef=null, uploadImageRef=null, removeImageRef=null ;
+
+        // prepare to save the initial values
+        let initialVals = null ;
+        function onReady() {
+            if ( ! initialVals )
+                initialVals = unloadVals() ;
+        }
 
         function doRender() {
 
+            // NOTE: We implement this as a function so that we can re-render the form after articles have been re-ordered.
+
             // initialize the image
-            let imageRef=null, uploadImageRef=null, removeImageRef=null ;
-            let imageUrl = gAppRef.makeFlaskUrl( "/images/publication/" + vals.pub_id ) ;
-            imageUrl += "?foo=" + Math.random() ; // FUDGE! To bypass the cache :-/
-            let onMissingImage = (evt) => {
+            let imageUrl ;
+            if ( initialVals )
+                imageUrl = imageRef.src ; // nb: leave whatever's there already
+            else {
+                imageUrl = gAppRef.makeFlaskUrl( "/images/publication/" + vals.pub_id ) ;
+                imageUrl += "?foo=" + Math.random() ; // FUDGE! To bypass the cache :-/
+            }
+            function onImageLoaded() { onReady() ; }
+            function onMissingImage() {
                 imageRef.src = "/images/placeholder.png" ;
                 removeImageRef.style.display = "none" ;
+                onReady() ;
             } ;
-            let onUploadImage = (evt) => {
+            function onUploadImage( evt ) {
                 if ( evt === null && !gAppRef.isFakeUploads() ) {
                     // nb: the publication image was clicked - trigger an upload request
                     uploadImageRef.click() ;
@@ -39,7 +55,7 @@ export class PublicationSearchResult2
                     imageData = data ;
                 } ) ;
             } ;
-            let onRemoveImage = () => {
+            function onRemoveImage() {
                 imageData = "{remove}" ;
                 imageRef.src = "/images/placeholder.png" ;
                 removeImageRef.style.display = "none" ;
@@ -107,9 +123,22 @@ export class PublicationSearchResult2
             const content = <div>
                 <div className="image-container">
                     <div className="row image">
-                        <img src={imageUrl} className="image" onError={onMissingImage} onClick={() => onUploadImage(null)} ref={r => imageRef=r} alt="Upload image." title="Click to upload an image for this publication." />
-                        <img src="/images/delete.png" className="remove-image" onClick={onRemoveImage} ref={r => removeImageRef=r} alt="Remove image." title="Remove the publication's image." />
-                        <input type="file" accept="image/*" onChange={onUploadImage} style={{display:"none"}} ref={r => uploadImageRef=r} />
+                        <img src={imageUrl} className="image"
+                            onLoad = {onImageLoaded}
+                            onError = {onMissingImage}
+                            onClick = { () => onUploadImage(null) }
+                            ref = { r => imageRef=r }
+                            alt="Upload image." title="Click to upload an image for this publication."
+                        />
+                        <img src="/images/delete.png" className="remove-image"
+                            onClick = {onRemoveImage}
+                            ref = { r => removeImageRef=r }
+                            alt="Remove image." title="Remove the publication's image."
+                        />
+                        <input type="file" accept="image/*" style={{display:"none"}}
+                            onChange = {onUploadImage}
+                            ref = { r => uploadImageRef=r }
+                        />
                     </div>
                 </div>
                 <div className="row name"> <label className="select top"> Name: </label>
@@ -155,6 +184,25 @@ export class PublicationSearchResult2
             return content ;
         }
 
+        function unloadVals() {
+            // unload the form values
+            let newVals = {} ;
+            for ( let r in refs ) {
+                if ( r === "publ_id" )
+                    newVals[ r ] = refs[r].state.value && refs[r].state.value.value ;
+                else if ( r === "pub_name" ) {
+                    if ( refs[r].state.value )
+                        newVals[ r ] = refs[r].state.value.label.trim() ;
+                } else if ( r === "pub_tags" ) {
+                    let tags = unloadCreatableSelect( refs[r] ) ;
+                    newVals[ r ] = tags.map( v => v.label ) ;
+                } else
+                    newVals[ r ] = refs[r].value.trim() ;
+            }
+            newVals._hasImage = ( removeImageRef.style.display !== "none" ) ;
+            return newVals ;
+        }
+
         function checkForDupe( vals ) {
             // check for an existing publication name/edition
             for ( let pub of Object.entries(gAppRef.caches.publications) ) {
@@ -190,20 +238,8 @@ export class PublicationSearchResult2
         // prepare the form buttons
         const buttons = {
             OK: () => {
-                // unload the new values
-                let newVals = {} ;
-                for ( let r in refs ) {
-                    if ( r === "publ_id" )
-                        newVals[ r ] = refs[r].state.value && refs[r].state.value.value ;
-                    else if ( r === "pub_name" ) {
-                        if ( refs[r].state.value )
-                            newVals[ r ] = refs[r].state.value.label.trim() ;
-                    } else if ( r === "pub_tags" ) {
-                        let vals = unloadCreatableSelect( refs[r] ) ;
-                        newVals[ r ] = vals.map( v => v.label ) ;
-                    } else
-                        newVals[ r ] = refs[r].value.trim() ;
-                }
+                // unload and validate the new values
+                let newVals = unloadVals() ;
                 if ( imageData ) {
                     newVals.imageData = imageData ;
                     newVals.imageFilename = imageFilename ;
@@ -225,15 +261,24 @@ export class PublicationSearchResult2
                     () => notify( newVals, refs )
                 ) ;
             },
-            Cancel: () => { gAppRef.closeModalForm() ; },
+            Cancel: () => {
+                let newVals = unloadVals() ;
+                if ( initialVals._hasImage && newVals._hasImage && imageData ) {
+                    // FUDGE! The image was changed, but we have no way to tell if it's the same image or not,
+                    // so we play it safe and force a confirmation.
+                    newVals._justDoIt = true ;
+                }
+                confirmDiscardChanges( initialVals, newVals,
+                    () => { gAppRef.closeModalForm() }
+                ) ;
+            },
         } ;
 
         // show the form
         const isNew = Object.keys( vals ).length === 0 ;
         gAppRef.showModalForm( "publication-form",
             isNew ? "New publication" : "Edit publication", "#e5f700",
-            doRender,
-            buttons
+            doRender, buttons
         ) ;
     }
 

web/src/PublisherSearchResult2.js

@@ -1,7 +1,7 @@
 import React from "react" ;
 import { gAppRef } from "./index.js" ;
 import { ImageFileUploader } from "./FileUploader.js" ;
-import { checkConstraints, ciCompare } from "./utils.js" ;
+import { checkConstraints, confirmDiscardChanges, ciCompare } from "./utils.js" ;
 
 // --------------------------------------------------------------------
 
@@ -10,18 +10,28 @@ export class PublisherSearchResult2
 
     static _doEditPublisher( vals, notify ) {
 
+        // initialize
         let refs = {} ;
 
+        // prepare to save the initial values
+        let initialVals = null ;
+        function onReady() {
+            if ( ! initialVals )
+                initialVals = unloadVals() ;
+        }
+
         // initialize the image
         let imageFilename=null, imageData=null ;
         let imageRef=null, uploadImageRef=null, removeImageRef=null ;
         let imageUrl = gAppRef.makeFlaskUrl( "/images/publisher/" + vals.publ_id ) ;
         imageUrl += "?foo=" + Math.random() ; // FUDGE! To bypass the cache :-/
-        let onMissingImage = (evt) => {
+        function onImageLoaded() { onReady() ; }
+        function onMissingImage() {
             imageRef.src = "/images/placeholder.png" ;
             removeImageRef.style.display = "none" ;
+            onReady() ;
         } ;
-        let onUploadImage = (evt) => {
+        function onUploadImage( evt ) {
             if ( evt === null && !gAppRef.isFakeUploads() ) {
                 // nb: the publisher image was clicked - trigger an upload request
                 uploadImageRef.click() ;
@@ -33,20 +43,33 @@ export class PublisherSearchResult2
                 imageData = data ;
             } ) ;
         } ;
-        let onRemoveImage = () => {
+        function onRemoveImage() {
             imageData = "{remove}" ;
             imageRef.src = "/images/placeholder.png" ;
             removeImageRef.style.display = "none" ;
-        } ;
+        }
 
         // prepare the form content
         /* eslint-disable jsx-a11y/img-redundant-alt */
         const content = <div>
             <div className="image-container">
                 <div className="row image">
-                    <img src={imageUrl} className="image" onError={onMissingImage} onClick={() => onUploadImage(null)} ref={r => imageRef=r} alt="Upload image." title="Click to upload an image for this publisher." />
-                    <img src="/images/delete.png" className="remove-image" onClick={onRemoveImage} ref={r => removeImageRef=r} alt="Remove image." title="Remove the publisher's image." />
-                    <input type="file" accept="image/*" onChange={onUploadImage} style={{display:"none"}} ref={r => uploadImageRef=r} />
+                    <img src={imageUrl} className="image"
+                        onLoad = {onImageLoaded}
+                        onError = {onMissingImage}
+                        onClick = { () => onUploadImage(null) }
+                        ref = { r => imageRef=r }
+                        alt="Upload image." title="Click to upload an image for this publisher."
+                    />
+                    <img src="/images/delete.png" className="remove-image"
+                        onClick = {onRemoveImage}
+                        ref = { r => removeImageRef=r }
+                        alt="Remove image." title="Remove the publisher's image."
+                    />
+                    <input type="file" accept="image/*" style={{display:"none"}}
+                        onChange = {onUploadImage}
+                        ref = { r => uploadImageRef=r }
+                    />
                 </div>
             </div>
             <div className="row name"> <label className="top"> Name: </label>
@@ -69,18 +92,24 @@ export class PublisherSearchResult2
             return false ;
         }
 
+        function unloadVals() {
+            // unload the form values
+            let newVals = {} ;
+            for ( let r in refs )
+                newVals[ r ] = refs[r].value.trim() ;
+            newVals._hasImage = ( removeImageRef.style.display !== "none" ) ;
+            return newVals ;
+        }
+
         // prepare the form buttons
         const buttons = {
             OK: () => {
-                // unload the new values
-                let newVals = {} ;
-                for ( let r in refs )
-                    newVals[ r ] = refs[r].value.trim() ;
+                // unload and validate the new values
+                let newVals = unloadVals() ;
                 if ( imageData ) {
                     newVals.imageData = imageData ;
                     newVals.imageFilename = imageFilename ;
                 }
-                // check the new values
                 const required = [
                     [ () => newVals.publ_name === "", "Please give them a name.", refs.publ_name ],
                     [ () => isNew && checkForDupe(newVals.publ_name), "There is already a publisher with this name.", refs.publ_name ],
@@ -92,12 +121,25 @@ export class PublisherSearchResult2
                     () => notify( newVals, refs )
                 ) ;
             },
-            Cancel: () => { gAppRef.closeModalForm() ; },
+            Cancel: () => {
+                let newVals = unloadVals() ;
+                if ( initialVals._hasImage && newVals._hasImage && imageData ) {
+                    // FUDGE! The image was changed, but we have no way to tell if it's the same image or not,
+                    // so we play it safe and force a confirmation.
+                    newVals._justDoIt = true ;
+                }
+                confirmDiscardChanges( initialVals, newVals,
+                    () => { gAppRef.closeModalForm() }
+                ) ;
+            },
         } ;
 
         // show the form
         const isNew = Object.keys( vals ).length === 0 ;
-        gAppRef.showModalForm( "publisher-form", isNew?"New publisher":"Edit publisher", "#eabe51", content, buttons ) ;
+        gAppRef.showModalForm( "publisher-form",
+            isNew ? "New publisher" : "Edit publisher", "#eabe51",
+            content, buttons
+        ) ;
     }
 
 }

web/src/utils.js

@@ -2,6 +2,8 @@ import React from "react" ;
 import ReactDOMServer from "react-dom/server" ;
 import { gAppRef } from "./index.js" ;
 
+const isEqual = require( "lodash.isequal" ) ;
+
 // --------------------------------------------------------------------
 
 export function checkConstraints( required, requiredCaption, optional, optionalCaption, accept ) {
@@ -55,6 +57,25 @@ export function checkConstraints( required, requiredCaption, optional, optionalC
     accept() ;
 }
 
+export function confirmDiscardChanges( oldVals, newVals, accept ) {
+    // check if confirmations have been disabled (for testing porpoises only)
+    if ( gAppRef.isDisableConfirmDiscardChanges() ) {
+        accept() ;
+        return ;
+    }
+    // check if the values have changed
+    if ( isEqual( oldVals, newVals ) ) {
+        // nope - just do it
+        accept() ;
+    } else {
+        // yup - ask the user to confirm first
+        gAppRef.ask( "Do you want to discard your changes?", "ask", {
+            OK: accept,
+            Cancel: null,
+        } ) ;
+    }
+}
+
 export function sortSelectableOptions( options ) {
     options.sort( (lhs,rhs) => {
         lhs = ReactDOMServer.renderToStaticMarkup( lhs.label ) ;