Issue 157: trigger onConfirm after set state

`onConfirm` is currently triggered before `setState` is called, to keep proper state with what is sent to `onConfirm`, `setState` should be called then `onConfirm`.
alphagov · Jun 14, 2017 · 2b24f4f · 2b24f4f
1 parent 7902859
commit 2b24f4f
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 18 deletions.
diff --git a/dist/accessible-autocomplete.min.js b/dist/accessible-autocomplete.min.js
diff --git a/dist/accessible-autocomplete.min.js.map b/dist/accessible-autocomplete.min.js.map
diff --git a/dist/lib/accessible-autocomplete.preact.min.js b/dist/lib/accessible-autocomplete.preact.min.js
diff --git a/dist/lib/accessible-autocomplete.preact.min.js.map b/dist/lib/accessible-autocomplete.preact.min.js.map
diff --git a/dist/lib/accessible-autocomplete.react.min.js b/dist/lib/accessible-autocomplete.react.min.js
diff --git a/dist/lib/accessible-autocomplete.react.min.js.map b/dist/lib/accessible-autocomplete.react.min.js.map
diff --git a/src/autocomplete.js b/src/autocomplete.js
@@ -156,7 +156,6 @@ export default class Autocomplete extends Component {
     let newQuery
     if (this.props.confirmOnBlur) {
       newQuery = newState.query || query
-      this.props.onConfirm(options[selected])
     } else {
       newQuery = query
     }
@@ -165,7 +164,7 @@ export default class Autocomplete extends Component {
       menuOpen: newState.menuOpen || false,
       query: newQuery,
       selected: null
-    })
+    }, this.props.confirmOnBlur ? this.props.onConfirm(newQuery) : null)
   }
 
   handleOptionBlur (evt, idx) {
@@ -257,13 +256,13 @@ export default class Autocomplete extends Component {
   handleOptionClick (evt, idx) {
     const selectedOption = this.state.options[idx]
     const newQuery = this.templateInputValue(selectedOption)
-    this.props.onConfirm(selectedOption)
+
     this.setState({
       focused: -1,
       menuOpen: false,
       query: newQuery,
       selected: -1
-    })
+    }, this.props.onConfirm(selectedOption))
   }
 
   handleOptionMouseDown (evt) {

diff --git a/test/functional/index.js b/test/functional/index.js
@@ -72,7 +72,7 @@ describe('Autocomplete', () => {
   })
 
   describe('behaviour', () => {
-    let autocomplete, autoselectAutocomplete, onConfirmAutocomplete, onConfirmTriggered,
+    let autocomplete, autoselectAutocomplete, onConfirmAutocomplete, onConfirmQuery,
       autoselectOnSelectAutocomplete, confirmOnBlurAutocomplete
 
     beforeEach(() => {
@@ -89,26 +89,27 @@ describe('Autocomplete', () => {
         source: suggest
       })
 
-      onConfirmTriggered = false
+      onConfirmQuery = null
+
       onConfirmAutocomplete = new Autocomplete({
         ...Autocomplete.defaultProps,
         id: 'test3',
-        onConfirm: () => { onConfirmTriggered = true },
+        onConfirm: query => { onConfirmQuery = query },
         source: suggest
       })
 
       autoselectOnSelectAutocomplete = new Autocomplete({
         ...Autocomplete.defaultProps,
         autoselect: true,
         id: 'test4',
-        onConfirm: () => { onConfirmTriggered = true },
+        onConfirm: query => { onConfirmQuery = query },
         source: suggest
       })
 
       confirmOnBlurAutocomplete = new Autocomplete({
         ...Autocomplete.defaultProps,
         id: 'test5',
-        onConfirm: () => { onConfirmTriggered = true },
+        onConfirm: query => { onConfirmQuery = query },
         confirmOnBlur: false,
         source: suggest
       })
@@ -223,7 +224,7 @@ describe('Autocomplete', () => {
           expect(autoselectOnSelectAutocomplete.state.focused).to.equal(null)
           expect(autoselectOnSelectAutocomplete.state.menuOpen).to.equal(false)
           expect(autoselectOnSelectAutocomplete.state.query).to.equal('France')
-          expect(onConfirmTriggered).to.equal(true)
+          expect(onConfirmQuery).to.equal(autoselectOnSelectAutocomplete.state.query)
         })
       })
 
@@ -234,7 +235,8 @@ describe('Autocomplete', () => {
           expect(confirmOnBlurAutocomplete.state.focused).to.equal(null)
           expect(confirmOnBlurAutocomplete.state.menuOpen).to.equal(false)
           expect(confirmOnBlurAutocomplete.state.query).to.equal('fr')
-          expect(onConfirmTriggered).to.equal(false)
+          expect(onConfirmQuery).to.not.equal(confirmOnBlurAutocomplete.state.query)
+          expect(onConfirmQuery).to.be.a('null')
         })
       })
     })
@@ -386,7 +388,7 @@ describe('Autocomplete', () => {
           expect(onConfirmAutocomplete.state.focused).to.equal(-1)
           expect(onConfirmAutocomplete.state.selected).to.equal(-1)
           expect(preventedDefault).to.equal(true)
-          expect(onConfirmTriggered).to.equal(true)
+          expect(onConfirmQuery).to.equal(onConfirmAutocomplete.state.query)
         })
       })