Commit da788daf authored by Robert Knight's avatar Robert Knight Committed by Kyle Keating

Remove checks for IE 11 in `TagEditor` component

`TagEditor` tested if the browser was IE 11 to determine whether to
check the `inputType` property of `InputEvent`. However it is not only
IE 11 that did not support this property.

From reading the comments in https://github.com/hypothesis/client/pull/1558 it
appears that the `inputType` checks was originally added to resolve an
issue with accepting suggestions by pressing Enter in an earlier version
of `TagEditor` that used the `<datalist>` element. As far as I can tell
this is no longer applicable. `<datalist>` was replaced by a custom UI
and accepting suggestions by pressing Enter works without this logic.
Therefore this commit removes it entirely.
parent 77478163
import { createElement } from 'preact'; import { createElement } from 'preact';
import { useMemo, useRef, useState } from 'preact/hooks'; import { useRef, useState } from 'preact/hooks';
import propTypes from 'prop-types'; import propTypes from 'prop-types';
import { withServices } from '../util/service-context'; import { withServices } from '../util/service-context';
import { isIE11 } from '../../shared/user-agent';
import AutocompleteList from './autocomplete-list'; import AutocompleteList from './autocomplete-list';
import { normalizeKeyName } from '../../shared/browser-compatibility-utils'; import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
...@@ -55,8 +54,6 @@ function TagEditor({ ...@@ -55,8 +54,6 @@ function TagEditor({
setSuggestionsListOpen(false); setSuggestionsListOpen(false);
}); });
const ie11 = useMemo(() => isIE11(), []);
/** /**
* Retrieve the current trimmed text value of the tag <input> * Retrieve the current trimmed text value of the tag <input>
*/ */
...@@ -120,20 +117,9 @@ function TagEditor({ ...@@ -120,20 +117,9 @@ function TagEditor({
} }
}; };
/** const handleOnInput = () => {
* Update the suggestions if the user changes the value of the input
*
* @param {import("preact").JSX.TargetedEvent<HTMLInputElement, InputEvent>} e
*/
const handleOnInput = e => {
onTagInput?.(pendingTag()); onTagInput?.(pendingTag());
if ( updateSuggestions();
e.inputType === 'insertText' ||
e.inputType === 'deleteContentBackward' ||
ie11 // inputType is not defined in IE 11, so trigger on any input in this case.
) {
updateSuggestions();
}
}; };
/** /**
......
...@@ -17,7 +17,6 @@ describe('TagEditor', function () { ...@@ -17,7 +17,6 @@ describe('TagEditor', function () {
let fakeOnAddTag; let fakeOnAddTag;
let fakeOnRemoveTag; let fakeOnRemoveTag;
let fakeOnTagInput; let fakeOnTagInput;
let fakeIsIE11;
function createComponent(props) { function createComponent(props) {
// Use an array of containers so we can test more // Use an array of containers so we can test more
...@@ -46,16 +45,10 @@ describe('TagEditor', function () { ...@@ -46,16 +45,10 @@ describe('TagEditor', function () {
fakeOnRemoveTag = sinon.stub(); fakeOnRemoveTag = sinon.stub();
fakeOnTagInput = sinon.stub(); fakeOnTagInput = sinon.stub();
fakeServiceUrl = sinon.stub().returns('http://serviceurl.com'); fakeServiceUrl = sinon.stub().returns('http://serviceurl.com');
fakeIsIE11 = sinon.stub().returns(false);
fakeTagsService = { fakeTagsService = {
filter: sinon.stub().returns(['tag4', 'tag3']), filter: sinon.stub().returns(['tag4', 'tag3']),
}; };
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({
'../../shared/user-agent': {
isIE11: fakeIsIE11,
},
});
}); });
afterEach(function () { afterEach(function () {
...@@ -91,12 +84,7 @@ describe('TagEditor', function () { ...@@ -91,12 +84,7 @@ describe('TagEditor', function () {
} }
// Simulates typing text // Simulates typing text
function typeInput(wrapper) { function typeInput(wrapper) {
if (!fakeIsIE11()) { wrapper.find('input').simulate('input', { inputType: 'insertText' });
wrapper.find('input').simulate('input', { inputType: 'insertText' });
} else {
// IE11 does not have an inputType key.
wrapper.find('input').simulate('input', {});
}
} }
it('adds appropriate tag values to the elements', () => { it('adds appropriate tag values to the elements', () => {
...@@ -417,58 +405,52 @@ describe('TagEditor', function () { ...@@ -417,58 +405,52 @@ describe('TagEditor', function () {
}); });
describe('navigating suggestions via keyboard', () => { describe('navigating suggestions via keyboard', () => {
[true, false].forEach(ie11 => { it('should set the initial `activeItem` value to -1 when opening suggestions', () => {
it('should set the initial `activeItem` value to -1 when opening suggestions', () => { const wrapper = createComponent();
fakeIsIE11.returns(ie11); wrapper.find('input').instance().value = 'non-empty';
const wrapper = createComponent(); typeInput(wrapper);
wrapper.find('input').instance().value = 'non-empty'; assert.equal(wrapper.find('AutocompleteList').prop('open'), true);
typeInput(wrapper); assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
assert.equal(wrapper.find('AutocompleteList').prop('open'), true); });
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
}); it('should increment the `activeItem` when pressing down circularly', () => {
const wrapper = createComponent();
it('should increment the `activeItem` when pressing down circularly', () => { wrapper.find('input').instance().value = 'non-empty';
fakeIsIE11.returns(ie11); typeInput(wrapper);
const wrapper = createComponent(); // 2 suggestions: ['tag3', 'tag4'];
wrapper.find('input').instance().value = 'non-empty'; navigateDown(wrapper);
typeInput(wrapper); assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
// 2 suggestions: ['tag3', 'tag4']; navigateDown(wrapper);
navigateDown(wrapper); assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1);
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0); navigateDown(wrapper);
navigateDown(wrapper); // back to unselected
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1); assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
navigateDown(wrapper); });
// back to unselected
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1); it('should decrement the `activeItem` when pressing up circularly', () => {
}); const wrapper = createComponent();
wrapper.find('input').instance().value = 'non-empty';
it('should decrement the `activeItem` when pressing up circularly', () => { typeInput(wrapper);
fakeIsIE11.returns(ie11); // 2 suggestions: ['tag3', 'tag4'];
const wrapper = createComponent(); navigateUp(wrapper);
wrapper.find('input').instance().value = 'non-empty'; assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1);
typeInput(wrapper); navigateUp(wrapper);
// 2 suggestions: ['tag3', 'tag4']; assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
navigateUp(wrapper); navigateUp(wrapper);
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1); assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
navigateUp(wrapper); });
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
navigateUp(wrapper); it('should set `activeItem` to -1 when clearing the suggestions', () => {
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1); const wrapper = createComponent();
}); wrapper.find('input').instance().value = 'non-empty';
typeInput(wrapper);
it('should set `activeItem` to -1 when clearing the suggestions', () => { navigateDown(wrapper);
fakeIsIE11.returns(ie11); // change to non-default value
const wrapper = createComponent(); assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
wrapper.find('input').instance().value = 'non-empty'; // clear suggestions
typeInput(wrapper); wrapper.find('input').instance().value = '';
navigateDown(wrapper); typeInput(wrapper);
// change to non-default value assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
// clear suggestions
wrapper.find('input').instance().value = '';
typeInput(wrapper);
assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
});
}); });
}); });
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment