Fix input focus on re-ordered elements (v10.1.6, #164)

integration-tests/regression.test.js

@@ -1,4 +1,4 @@
-const { getByText, fireEvent, waitFor, getByLabelText } = require('@testing-library/dom')
+const { getByText, queryAllByText, fireEvent, waitFor, getByLabelText } = require('@testing-library/dom')
 const { default: userEvent } = require('@testing-library/user-event')
 const { startApp } = require('./test-app')
 
@@ -221,4 +221,44 @@ describe('Tram-One - regressions', () => {
 		// cleanup - remove app
 		appContainer.remove()
 	})
+
+	it('should not error when reseting focus if the number of elements changed', async () => {
+		// for focus to work correctly, the element needs to be attached to the document
+		const appContainer = document.createElement('div')
+		appContainer.id = 'app'
+		window.document.body.appendChild(appContainer)
+
+		// start the app using a css selector
+		startApp('#app')
+
+		// previously when interacting with an input, if the number of elements decreased
+		// an error was thrown because the element to focus on no longer existed
+
+		// focus on the child input
+		userEvent.click(getByLabelText(appContainer, 'Sub Mirror Input'))
+
+		// verify that the element has focus (before we start changing text)
+		await waitFor(() => {
+			expect(getByLabelText(appContainer, 'Sub Mirror Input')).toHaveFocus()
+		})
+
+		// update the state by typing
+		userEvent.type(getByLabelText(appContainer, 'Sub Mirror Input'), 'Test')
+
+		// verify the element has the new value
+		expect(getByLabelText(appContainer, 'Sub Mirror Input')).toHaveValue('Test')
+
+		// verify the element and it's parent have the new value
+		// also verify that the elements were added above it too (previously this would have failed)
+		// the element should still have focus
+		await waitFor(() => {
+			expect(getByLabelText(appContainer, 'Sub Mirror Input')).toHaveFocus()
+			expect(queryAllByText(appContainer, '-')).toHaveLength(4)
+			expect(getByLabelText(appContainer, 'Mirror Input')).toHaveValue('Test')
+			expect(getByLabelText(appContainer, 'Sub Mirror Input')).toHaveValue('Test')
+		})
+
+		// cleanup - remove app
+		appContainer.remove()
+	})
 })

integration-tests/test-app/sub-mirror-input.js

@@ -8,14 +8,17 @@ const html = registerHtml({
  * component to test url parameters
  */
 module.exports = () => {
-	const mirrorable = useGlobalStore('mirrorable-input', { value: '' })
+	const mirrorable = useGlobalStore('mirrorable-input')
 
 	const onEvent = event => {
 		mirrorable.value = event.target.value
 	}
 
+	const letterSpans = [...new Array(mirrorable.value.length)].map(() => html`<span class="letter-span">-</span>`)
+
 	return html`
 		<div>
+			${letterSpans}
 			<label for="sub-mirrorable-input">Sub Mirror Input</label>
 			<input id="sub-mirrorable-input" class="main input-component" type="text" value=${mirrorable.value} onkeyup=${onEvent} />
 		</div>

package.json

@@ -1,6 +1,6 @@
 {
   "name": "tram-one",
-  "version": "10.1.5",
+  "version": "10.1.6",
   "description": "🚋 Modern View Framework for Vanilla Javascript",
   "main": "dist/tram-one.cjs.js",
   "commonjs": "dist/tram-one.cjs.js",

src/observe-tag.js

@@ -1,6 +1,24 @@
 const { observe } = require('@nx-js/observer-util')
 const { TRAM_TAG_REACTION, TRAM_TAG_NEW_EFFECTS, TRAM_TAG_CLEANUP_EFFECTS } = require('./node-names')
 
+// functions to go to nodes or indicies (made for .map)
+const toIndicies = (node, index) => index
+const toNodes = allNodes => index => allNodes[index]
+
+// sorting function that prioritizes indicies that are closest to a target
+// e.g. target = 3, [1, 2, 3, 4, 5] => [3, 2, 4, 1, 5]
+const byDistanceFromIndex = targetIndex => (indexA, indexB) => {
+	const diffFromTargetA = Math.abs(indexA - targetIndex)
+	const diffFromTargetB = Math.abs(indexB - targetIndex)
+	return diffFromTargetA - diffFromTargetB
+}
+
+// get an array including the element and all it's children
+const parentAndChildrenElements = node => {
+	const children = node.querySelectorAll('*')
+	return [node, ...children]
+}
+
 /**
  * This is a helper function for the dom creation.
  * This function observes any state values used when making the tag, and allow it to update
@@ -15,24 +33,32 @@ module.exports = tagFunction => {
 		let oldTag = tagResult
 		const removedElementWithFocusData = {
 			index: null,
+			tagName: null,
 			selectionStart: null,
 			selectionEnd: null,
-			selectionDirection: null
+			selectionDirection: null,
+			scrollLeft: null,
+			scrollTop: null
 		}
 
 		// remove oldTag first so that we unobserve before we re-observe
 		if (oldTag) {
 			// if there was focus, we need to figure out what element has it
-			const children = oldTag.querySelectorAll('*')
-			const parentAndChildrenNodes = [oldTag, ...children]
-			removedElementWithFocusData.index = parentAndChildrenNodes.findIndex(element => element === document.activeElement)
+			const allElements = parentAndChildrenElements(oldTag)
+			removedElementWithFocusData.index = allElements.findIndex(element => element === document.activeElement)
 
 			// if an element had focus, copy over all the selection data (so we can copy it back later)
 			if (removedElementWithFocusData.index >= 0) {
-				const removedElementWithFocus = parentAndChildrenNodes[removedElementWithFocusData.index]
+				// get the actual element
+				const removedElementWithFocus = allElements[removedElementWithFocusData.index]
+
+				// copy over the data
+				removedElementWithFocusData.tagName = removedElementWithFocus.tagName
 				removedElementWithFocusData.selectionStart = removedElementWithFocus.selectionStart
 				removedElementWithFocusData.selectionEnd = removedElementWithFocus.selectionEnd
 				removedElementWithFocusData.selectionDirection = removedElementWithFocus.selectionDirection
+				removedElementWithFocusData.scrollLeft = removedElementWithFocus.scrollLeft
+				removedElementWithFocusData.scrollTop = removedElementWithFocus.scrollTop
 			}
 
 			const emptyDiv = document.createElement('div')
@@ -56,19 +82,35 @@ module.exports = tagFunction => {
 
 			// if an element had focus, reapply it
 			if (removedElementWithFocusData.index >= 0) {
-				const children = tagResult.querySelectorAll('*')
-
-				const elementToGiveFocus = [tagResult, ...children][removedElementWithFocusData.index]
-				elementToGiveFocus.focus()
-
-				// also try to set the selection, if there is a selection for this element
-				const hasSelectionStart = removedElementWithFocusData.selectionStart !== null && removedElementWithFocusData.selectionStart !== undefined
-				if (hasSelectionStart) {
-					elementToGiveFocus.setSelectionRange(
-						removedElementWithFocusData.selectionStart,
-						removedElementWithFocusData.selectionEnd,
-						removedElementWithFocusData.selectionDirection
-					)
+				const allElements = parentAndChildrenElements(tagResult)
+
+				// we'll look through the elements (in order of nodes closest to original index) and find a tag that matches.
+				// this means if it didn't move, we'll get it right away,
+				// if it did, we'll look at the elements closest to the original position
+				const nodeMatchesTagName = node => node.tagName === removedElementWithFocusData.tagName
+				const elementToGiveFocus = allElements
+					.map(toIndicies)
+					.sort(byDistanceFromIndex(removedElementWithFocusData.index))
+					.map(toNodes(allElements))
+					.find(nodeMatchesTagName)
+
+				// if the element / child exists, focus it
+				if (elementToGiveFocus !== undefined) {
+					elementToGiveFocus.focus()
+
+					// also try to set the selection, if there is a selection for this element
+					const hasSelectionStart = removedElementWithFocusData.selectionStart !== null && removedElementWithFocusData.selectionStart !== undefined
+					if (hasSelectionStart) {
+						elementToGiveFocus.setSelectionRange(
+							removedElementWithFocusData.selectionStart,
+							removedElementWithFocusData.selectionEnd,
+							removedElementWithFocusData.selectionDirection
+						)
+					}
+
+					// also set the scrollLeft and scrollTop (since this is reset to 0 by default)
+					elementToGiveFocus.scrollLeft = removedElementWithFocusData.scrollLeft
+					elementToGiveFocus.scrollTop = removedElementWithFocusData.scrollTop
 				}
 			}