From bf8549f861d7aba83f708852307b7924cd90c85d Mon Sep 17 00:00:00 2001 From: Taka Date: Tue, 4 Feb 2020 10:04:18 +0000 Subject: [PATCH] Tightened up how we manage input focus. --- web/src/App.js | 36 ++++++++++++++++++----------- web/src/ArticleSearchResult2.js | 14 +++++------ web/src/PublicationSearchResult2.js | 12 +++++----- web/src/PublisherSearchResult2.js | 4 ++-- web/src/SearchForm.js | 6 ++--- web/src/utils.js | 20 +++++++++++----- 6 files changed, 54 insertions(+), 38 deletions(-) diff --git a/web/src/App.js b/web/src/App.js index 6a7f2a2..47666b5 100644 --- a/web/src/App.js +++ b/web/src/App.js @@ -44,6 +44,7 @@ export default class App extends React.Component // initialize this._searchFormRef = React.createRef() ; this._modalFormRef = React.createRef() ; + this._setFocusTo = null ; // figure out the base URL of the Flask backend server // NOTE: We allow the caller to do this since the test suite will usually spin up @@ -137,6 +138,17 @@ export default class App extends React.Component window.addEventListener( "keydown", this.onKeyDown.bind( this ) ) ; } + componentDidUpdate() { + // we've finished rendering the page, check if we should set focus + if ( this._setFocusTo ) { + if ( this._setFocusTo.current ) + this._setFocusTo.current.focus() ; + else + this._setFocusTo.focus() ; + } + this._setFocusTo = null ; + } + componentWillUnmount() { // clean up window.removeEventListener( "keydown", this.onKeyDown ) ; @@ -145,9 +157,9 @@ export default class App extends React.Component onSearch( query ) { // run the search query = query.trim() ; + const queryStringRef = this._searchFormRef.current.queryStringRef.current ; if ( query.length === 0 ) { - this.focusQueryString() ; - this.showErrorMsg( "Please enter something to search for." ) + this.showErrorMsg( "Please enter something to search for.", queryStringRef ) return ; } axios.post( this.makeFlaskUrl( "/search" ), { @@ -155,8 +167,8 @@ export default class App extends React.Component no_hilite: this._disableSearchResultHighlighting, } ) .then( resp => { + this._setFocusTo = queryStringRef ; this.setState( { searchResults: resp.data, searchSeqNo: this.state.searchSeqNo+1 } ) ; - this.focusQueryString() ; } ) .catch( err => { this.showErrorResponse( "The search query failed", err ) ; @@ -198,8 +210,8 @@ export default class App extends React.Component } closeModalForm() { + this._setFocusTo = this._searchFormRef.current.queryStringRef ; this.setState( { modalForm: null } ) ; - setTimeout( () => { this.focusQueryString() ; }, 100 ) ; } showInfoToast( msg ) { this._doShowToast( "info", msg, 5*1000 ) ; } @@ -229,17 +241,19 @@ export default class App extends React.Component else content =
{err.response.data}
; } + const msg = err.response ? err.response.statusText : err ; const buttons = { Close: () => this.closeModalForm() } ; - this.showModalForm( "error-response", err.response.statusText, "red", + this.showModalForm( "error-response", msg, "red",
{caption}: {content}
, buttons ) ; } - showErrorMsg( content ) { + showErrorMsg( content, setFocusTo ) { // show the error message in a modal dialog this.ask( content, "error", - { "OK": null } + { "OK": null }, + setFocusTo ) ; } @@ -247,7 +261,7 @@ export default class App extends React.Component this.showWarningToast( makeSmartBulletList( caption, warnings ) ) ; } - ask( content, iconType, buttons ) { + ask( content, iconType, buttons, setFocusTo ) { // prepare the buttons let buttons2 = [] ; for ( let b in buttons ) { @@ -257,6 +271,7 @@ export default class App extends React.Component if ( notify ) notify() ; // dismiss the dialog + this._setFocusTo = setFocusTo ? setFocusTo : this._searchFormRef.current.queryStringRef ; this.setState( { askDialog: null } ) ; } ; } @@ -337,11 +352,6 @@ export default class App extends React.Component this.setState( { startupTasks: this.state.startupTasks } ) ; } - focusQueryString() { - if ( this._searchFormRef.current ) - this._searchFormRef.current.focusQueryString() ; - } - isTestMode() { return process.env.REACT_APP_TEST_MODE ; } isDisableConstraints() { return this._disableConstraints ; } isFakeUploads() { return this._fakeUploads ; } diff --git a/web/src/ArticleSearchResult2.js b/web/src/ArticleSearchResult2.js index 9fd17ee..0eb7b5e 100644 --- a/web/src/ArticleSearchResult2.js +++ b/web/src/ArticleSearchResult2.js @@ -192,15 +192,15 @@ export class ArticleSearchResult2 } // check the new values const required = [ - [ () => newVals.article_title === "", "Please give it a title." ], + [ () => newVals.article_title === "", "Please give it a title.", refs.article_title ], ] ; const optional = [ - [ () => newVals.pub_id === null, "No publication was specified." ], - [ () => newVals.article_pageno === "" && newVals.pub_id !== null, "No page number was specified." ], - [ () => newVals.article_pageno !== "" && newVals.pub_id === null, "A page number was specified but no publication." ], - [ () => newVals.article_pageno !== "" && !isNumeric(newVals.article_pageno), "The page number is not numeric." ], - [ () => newVals.article_snippet === "", "No snippet was provided." ], - [ () => newVals.article_authors.length === 0, "No authors were specified." ], + [ () => newVals.pub_id === null, "No publication was specified.", refs.pub_id ], + [ () => newVals.article_pageno === "" && newVals.pub_id !== null, "No page number was specified.", refs.article_pageno ], + [ () => newVals.article_pageno !== "" && newVals.pub_id === null, "A page number was specified but no publication.", refs.pub_id ], + [ () => newVals.article_pageno !== "" && !isNumeric(newVals.article_pageno), "The page number is not numeric.", refs.article_pageno ], + [ () => newVals.article_snippet === "", "No snippet was provided.", refs.article_snippet ], + [ () => newVals.article_authors.length === 0, "No authors were specified.", refs.article_authors ], ] ; const verb = isNew ? "create" : "update" ; checkConstraints( diff --git a/web/src/PublicationSearchResult2.js b/web/src/PublicationSearchResult2.js index 1926aa5..dc0aa3b 100644 --- a/web/src/PublicationSearchResult2.js +++ b/web/src/PublicationSearchResult2.js @@ -209,14 +209,14 @@ export class PublicationSearchResult2 newVals.imageFilename = imageFilename ; } const required = [ - [ () => newVals.pub_name === undefined, "Please give it a name." ], - [ () => isNew && checkForDupe(newVals), "There is already a publication with this name/edition." ], + [ () => newVals.pub_name === undefined, "Please give it a name.", refs.pub_name ], + [ () => isNew && checkForDupe(newVals), "There is already a publication with this name/edition.", refs.pub_edition ], ] ; const optional = [ - [ () => newVals.pub_name !== undefined && newVals.pub_edition === "", "The publication's edition was not specified." ], - [ () => newVals.pub_date === "", "The publication date was not specified." ], - [ () => newVals.publ_id === null, "A publisher was not specified." ], - [ () => checkArticlePageNumbers(articles), "Some article page numbers are out of order." ], + [ () => newVals.pub_name !== undefined && newVals.pub_edition === "", "The publication's edition was not specified.", refs.pub_edition ], + [ () => newVals.pub_date === "", "The publication date was not specified.", refs.pub_date ], + [ () => newVals.publ_id === null, "A publisher was not specified.", refs.publ_id ], + [ () => checkArticlePageNumbers(articles), "Some article page numbers are out of order.", null ], ] ; const verb = isNew ? "create" : "update" ; checkConstraints( diff --git a/web/src/PublisherSearchResult2.js b/web/src/PublisherSearchResult2.js index fbff65e..9ccee2a 100644 --- a/web/src/PublisherSearchResult2.js +++ b/web/src/PublisherSearchResult2.js @@ -82,8 +82,8 @@ export class PublisherSearchResult2 } // check the new values const required = [ - [ () => newVals.publ_name === "", "Please give them a name." ], - [ () => isNew && checkForDupe(newVals.publ_name), "There is already a publisher with this name." ], + [ () => 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 ], ] ; const verb = isNew ? "create" : "update" ; checkConstraints( diff --git a/web/src/SearchForm.js b/web/src/SearchForm.js index 509d1d1..4e71df1 100644 --- a/web/src/SearchForm.js +++ b/web/src/SearchForm.js @@ -14,7 +14,7 @@ export default class SearchForm extends React.Component } ; // initialize - this._queryStringRef = React.createRef() ; + this.queryStringRef = React.createRef() ; } render() { @@ -24,7 +24,7 @@ export default class SearchForm extends React.Component this.setState( { queryString: e.target.value } ) } - ref = {this._queryStringRef} + ref = {this.queryStringRef} autoFocus />