Skip to content

Commit

Permalink
Fix case-sensitivity issue with rendering suggested tags.
Browse files Browse the repository at this point in the history
The `TagEditor` component formerly did not take casing into account
in its formatting function for suggested tags. The match should be
case-insensitive in the formatter, as it is in the service that does
the filtering of tags.

This is fixed in two ways:

1. Make substring matching in the formatting function case-insensitive.
   Render the substring match according to the suggested tag's casing.
   Fixes this issue specifically.
2. Provide a fallback in the formatter for when the input text does not
   "seem" to match the suggested `item`. In these cases, just render the
   suggested tag as-is. This will prevent the formatter from spazzing
   out if its notion of matching differs from the tag-service's in
   any future case.

Fixes #2547
  • Loading branch information
lyzadanger committed Sep 21, 2020
1 parent 478c173 commit 6bd09e7
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 6 deletions.
29 changes: 23 additions & 6 deletions src/sidebar/components/tag-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,32 @@ function TagEditor({
* @param {string} item - Suggested tag
* @return {JSXElement} - Formatted tag for use in list
*/
const formatSuggestItem = item => {
const curVal = pendingTag();
const prefix = item.slice(0, item.indexOf(curVal));
const suffix = item.slice(item.indexOf(curVal) + curVal.length);
const formatSuggestedItem = item => {
// filtering of tags is case-insensitive
const curVal = pendingTag().toLowerCase();
const suggestedTag = item.toLowerCase();
const matchIndex = suggestedTag.indexOf(curVal);

// If the current input doesn't seem to match the suggested tag,
// just render the tag as-is.
if (matchIndex === -1) {
return <span>{item}</span>;
}

// Break the suggested tag into three parts:
// 1. Substring of the suggested tag that occurs before the match
// with the current input
const prefix = item.slice(0, matchIndex);
// 2. Substring of the suggested tag that matches the input text. NB:
// This may be in a different case than the input text.
const matchString = item.slice(matchIndex, matchIndex + curVal.length);
// 3. Substring of the suggested tag that occurs after the matched input
const suffix = item.slice(matchIndex + curVal.length);

return (
<span>
<strong>{prefix}</strong>
{curVal}
{matchString}
<strong>{suffix}</strong>
</span>
);
Expand Down Expand Up @@ -324,7 +341,7 @@ function TagEditor({
<AutocompleteList
id={`${tagEditorId}-autocomplete-list`}
list={suggestions}
listFormatter={formatSuggestItem}
listFormatter={formatSuggestedItem}
open={suggestionsListOpen}
onSelectItem={handleSelect}
itemPrefixId={`${tagEditorId}-autocomplete-list-item-`}
Expand Down
50 changes: 50 additions & 0 deletions src/sidebar/components/test/tag-editor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,56 @@ describe('TagEditor', function () {
assert.equal(wrapper.find('AutocompleteList').prop('list')[1], 'tag4');
});

it('shows case-insensitive matches to suggested tags', () => {
fakeTagsService.filter.returns(['fine AArdvark', 'AAArgh']);
const wrapper = createComponent();
wrapper.find('input').instance().value = 'aa';
typeInput(wrapper);

const formattingFn = wrapper.find('AutocompleteList').prop('listFormatter');
const tagList = wrapper.find('AutocompleteList').prop('list');

const firstSuggestedTag = mount(formattingFn(tagList[0]))
.find('span')
.text();
const secondSuggestedTag = mount(formattingFn(tagList[1]))
.find('span')
.text();

// Even though the entered text was lower case ('aa'), the suggested tag
// should be rendered with its original casing (upper-case here)
assert.equal(firstSuggestedTag, 'AAArgh');
assert.equal(secondSuggestedTag, 'fine AArdvark');
});

it('shows suggested tags as-is if they do not seem to match the input', () => {
// This case addresses a situation in which a substring match isn't found
// for the current input text against a given suggested tag. This should not
// happen in practice—i.e. filtered tags should match the current input—
// but there is no contract that the tags service filtering uses the same
// "matching" as the component, so we should be able to handle cases where
// there doesn't "seem" to be a match by just rendering the suggested tag
// as-is.
fakeTagsService.filter.returns(['fine AArdvark', 'AAArgh']);
const wrapper = createComponent();
wrapper.find('input').instance().value = 'bb';
typeInput(wrapper);

const formattingFn = wrapper.find('AutocompleteList').prop('listFormatter');
const tagList = wrapper.find('AutocompleteList').prop('list');

const firstSuggestedTag = mount(formattingFn(tagList[0]))
.find('span')
.text();
const secondSuggestedTag = mount(formattingFn(tagList[1]))
.find('span')
.text();

// Obviously, these don't have a `bb` substring; we'll just render them...
assert.equal(firstSuggestedTag, 'AAArgh');
assert.equal(secondSuggestedTag, 'fine AArdvark');
});

it('passes the text value to filter() after receiving input', () => {
const wrapper = createComponent();
wrapper.find('input').instance().value = 'tag3';
Expand Down

0 comments on commit 6bd09e7

Please sign in to comment.