Skip to content

Commit

Permalink
Fix recursive search being triggered (fixes #20)
Browse files Browse the repository at this point in the history
See code comments in this commit.
  • Loading branch information
haukex committed Sep 22, 2024
1 parent 373d42b commit 1bbaadd
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 20 deletions.
4 changes: 2 additions & 2 deletions src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ <h1>German-English Dictionary</h1>
</div>

<p>
<!-- Note it's important the input box is disabled by default! -->
<input type="text" id="search-term" placeholder="🔍 Search" disabled>
<!-- Note it's important the input box is readonly by default! -->
<input type="text" id="search-term" placeholder="🔍 Search" readonly>
<progress id="search-progress" max="100" class="d-none"></progress>
</p>
<figure><!-- for horiz scroll (simple.css) -->
Expand Down
20 changes: 9 additions & 11 deletions src/js/keyboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,33 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

//TODO: As a workaround for Edge and Chrome, I've commented out the Enter key stuff...

/** Adds keyboard handlers to the search field. */
export function initInputFieldKeys(elem :HTMLInputElement, _onEnter :()=>void) {
export function initInputFieldKeys(elem :HTMLInputElement, onEnter :()=>void) {
elem.addEventListener('keyup', event => {
// Escape key clears input
if (event.key=='Escape') {
elem.value = ''
}
// Enter key triggers search
/*else if (event.key=='Enter') {
else if (event.key=='Enter') {
event.preventDefault()
event.stopPropagation()
onEnter()
}*/
}
})
/* 'Enter' is handled in keyup above, but we still need to prevent all of its default
* behavior here so it doesn't fire the "change" event and cause the search to run twice. */
elem.addEventListener('keydown', _event => {
/*if (event.key=='Enter') {
elem.addEventListener('keydown', event => {
if (event.key=='Enter') {
event.preventDefault()
event.stopPropagation()
}*/
}
})
// keypress is deprecated, we'll include it anyway for now
elem.addEventListener('keypress', _event => {
/*if (event.key=='Enter') {
elem.addEventListener('keypress', event => {
if (event.key=='Enter') {
event.preventDefault()
event.stopPropagation()
}*/
}
})
}
14 changes: 7 additions & 7 deletions src/js/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,12 @@ window.addEventListener('DOMContentLoaded', async () => {
// enable/disable UI components depending on state
if ( newState === MainState.Ready ) {
rand_entry_link.classList.remove('busy-link')
search_term.removeAttribute('disabled')
search_term.removeAttribute('readonly')
}
else {
search_term.setAttribute('disabled','disabled')
/* DON'T use `disabled`, because in the case where this code is going to the state `Searching` due to a search,
* setting that attribute causes a recursive `change` event and search to be fired here! (Chrome and Edge) */
search_term.setAttribute('readonly','readonly')
// note the click handler must check the state too and ignore clicks when not Ready
rand_entry_link.classList.add('busy-link')
}
Expand All @@ -108,9 +110,9 @@ window.addEventListener('DOMContentLoaded', async () => {
}
search_progress.classList.add('d-none')
state = newState
console.debug(`updateState done, state=${MainState[state]}`)
//console.debug(`updateState done, state=${MainState[state]}`)
}
// call this immediately (note the input box should already be disabled in HTML, but there are other things to update)
// call this immediately (note the input box should already be readonly in HTML, but there are other things to update)
updateState(state)

// utility function to clear the results table
Expand Down Expand Up @@ -232,7 +234,6 @@ window.addEventListener('DOMContentLoaded', async () => {
// this is our handler for running the search:
const doSearch = (rawWhat: string, fromInputNotUrl :boolean) => {
console.debug(`doSearch for '${rawWhat}' (fromInputNotUrl=${fromInputNotUrl}, state=${MainState[state]})`)
//TODO: In Chrome and Edge, the following state guard is not working?! (Enter key causes two searches, both of which see state===Ready for some reason!?)
if ( state !== MainState.Ready ) return

const what = cleanSearchTerm(rawWhat)
Expand Down Expand Up @@ -283,7 +284,6 @@ window.addEventListener('DOMContentLoaded', async () => {
})

// Install event listener for input field changes
//TODO: In Chrome and Edge, hitting Enter causes both of these to fire?! (temporary workaround in keyboard.ts)
search_term.addEventListener('change', () => {
console.debug(`Search from input field change event for '${search_term.value}'`)
doSearch(search_term.value, true)
Expand Down Expand Up @@ -325,7 +325,7 @@ window.addEventListener('DOMContentLoaded', async () => {
updateState(MainState.Ready)
// Install event listener for browser navigation updating the URL hash
window.addEventListener('hashchange', searchFromUrl)
// Trigger a search upon loading (the input field was disabled until now)
// Trigger a search upon loading (the input field was readonly until now, so we know the user didn't enter anything there)
searchFromUrl() // may transition to `Searching`!
// TypeScript doesn't realize that `state` may have changed here.
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
Expand Down

0 comments on commit 1bbaadd

Please sign in to comment.