From 0d56e00903124004fb50f56787e4ce74d607b5c7 Mon Sep 17 00:00:00 2001 From: Taka Date: Sat, 8 Feb 2020 05:07:21 +0000 Subject: [PATCH] Added confirmation for discarding changes made to a form. --- asl_articles/tests/test_articles.py | 45 +++++++- asl_articles/tests/test_publications.py | 37 +++++- asl_articles/tests/test_publishers.py | 22 +++- asl_articles/tests/utils.py | 145 +++++++++++++++++++++++- web/package-lock.json | 5 + web/package.json | 1 + web/src/App.js | 8 +- web/src/ArticleSearchResult2.js | 118 ++++++++++++------- web/src/ModalForm.js | 8 +- web/src/PublicationSearchResult2.js | 99 +++++++++++----- web/src/PublisherSearchResult2.js | 72 +++++++++--- web/src/utils.js | 21 ++++ 12 files changed, 474 insertions(+), 107 deletions(-) diff --git a/asl_articles/tests/test_articles.py b/asl_articles/tests/test_articles.py index 05d7c5f..b465366 100644 --- a/asl_articles/tests/test_articles.py +++ b/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 ) diff --git a/asl_articles/tests/test_publications.py b/asl_articles/tests/test_publications.py index 82f38d3..60cfea8 100644 --- a/asl_articles/tests/test_publications.py +++ b/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 ) diff --git a/asl_articles/tests/test_publishers.py b/asl_articles/tests/test_publishers.py index 3806eea..9939e8a 100644 --- a/asl_articles/tests/test_publishers.py +++ b/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 ) diff --git a/asl_articles/tests/utils.py b/asl_articles/tests/utils.py index aea179f..b255c67 100644 --- a/asl_articles/tests/utils.py +++ b/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() diff --git a/web/package-lock.json b/web/package-lock.json index a464e65..1ba2e87 100644 --- a/web/package-lock.json +++ b/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", diff --git a/web/package.json b/web/package.json index 4a28355..5656122 100644 --- a/web/package.json +++ b/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", diff --git a/web/src/App.js b/web/src/App.js index 47666b5..a86e2b5 100644 --- a/web/src/App.js +++ b/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 (
{content} { this.state.modalForm !== null && } { this.state.askDialog !== null && - + } { this._storeMsgs &&
@@ -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) diff --git a/web/src/ArticleSearchResult2.js b/web/src/ArticleSearchResult2.js index 0eb7b5e..424affe 100644 --- a/web/src/ArticleSearchResult2.js +++ b/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 =
- onUploadImage(null)} ref={r => imageRef=r} alt="Upload image." title="Click to upload an image for this article." /> - removeImageRef=r} alt="Remove image." title="Remove the article's image." /> - uploadImageRef=r} /> + onUploadImage(null) } + ref = { r => imageRef=r } + alt="Upload image." title="Click to upload an image for this article." + /> + removeImageRef=r } + alt="Remove image." title="Remove the article's image." + /> + uploadImageRef=r } + />
@@ -154,43 +176,48 @@ export class ArticleSearchResult2
; + 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 + ) ; } } diff --git a/web/src/ModalForm.js b/web/src/ModalForm.js index ba3557d..171119f 100644 --- a/web/src/ModalForm.js +++ b/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} ) ; } - return ( + + // show the dialog + return ( {this.props.title} { typeof this.props.content === "function" ? this.props.content() : this.props.content } diff --git a/web/src/PublicationSearchResult2.js b/web/src/PublicationSearchResult2.js index dc0aa3b..c924ee4 100644 --- a/web/src/PublicationSearchResult2.js +++ b/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 =
- onUploadImage(null)} ref={r => imageRef=r} alt="Upload image." title="Click to upload an image for this publication." /> - removeImageRef=r} alt="Remove image." title="Remove the publication's image." /> - uploadImageRef=r} /> + onUploadImage(null) } + ref = { r => imageRef=r } + alt="Upload image." title="Click to upload an image for this publication." + /> + removeImageRef=r } + alt="Remove image." title="Remove the publication's image." + /> + uploadImageRef=r } + />
@@ -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 ) ; } diff --git a/web/src/PublisherSearchResult2.js b/web/src/PublisherSearchResult2.js index 9ccee2a..0021554 100644 --- a/web/src/PublisherSearchResult2.js +++ b/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 =
- onUploadImage(null)} ref={r => imageRef=r} alt="Upload image." title="Click to upload an image for this publisher." /> - removeImageRef=r} alt="Remove image." title="Remove the publisher's image." /> - uploadImageRef=r} /> + onUploadImage(null) } + ref = { r => imageRef=r } + alt="Upload image." title="Click to upload an image for this publisher." + /> + removeImageRef=r } + alt="Remove image." title="Remove the publisher's image." + /> + uploadImageRef=r } + />
@@ -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 + ) ; } } diff --git a/web/src/utils.js b/web/src/utils.js index 462ff5f..ea0e0f8 100644 --- a/web/src/utils.js +++ b/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 ) ;