-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tag suggestions #4034
Tag suggestions #4034
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,16 @@ | ||
'use strict'; | ||
|
||
var escapeHtml = require('escape-html'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need a new line after the vendor bundle list. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol I did that and then removed it thinking you would say not to have two lines :P |
||
|
||
var Controller = require('../base/controller'); | ||
var LozengeController = require('./lozenge-controller'); | ||
var AutosuggestDropdownController = require('./autosuggest-dropdown-controller'); | ||
var SearchTextParser = require('../util/search-text-parser'); | ||
var stringUtil = require('../util/string'); | ||
|
||
const FACET_TYPE = 'FACET'; | ||
const TAG_TYPE = 'TAG'; | ||
const MAX_SUGGESTIONS = 5; | ||
|
||
/** | ||
* Controller for the search bar. | ||
|
@@ -15,107 +22,11 @@ class SearchBarController extends Controller { | |
this._input = this.refs.searchBarInput; | ||
this._lozengeContainer = this.refs.searchBarLozenges; | ||
|
||
let explanationList = [ | ||
{ | ||
title: 'user:', | ||
explanation: 'search by username', | ||
}, | ||
{ | ||
title: 'tag:', | ||
explanation: 'search for annotations with a tag', | ||
}, | ||
{ | ||
title: 'url:', | ||
explanation: 'see all annotations on a page', | ||
}, | ||
{ | ||
title: 'group:', | ||
explanation: 'show annotations created in a group you are a member of', | ||
}, | ||
]; | ||
|
||
var selectFacet = facet => { | ||
this._input.value = facet; | ||
|
||
setTimeout(()=>{ | ||
this._input.focus(); | ||
}, 0); | ||
}; | ||
|
||
|
||
var getTrimmedInputValue = () => { | ||
return this._input.value.trim(); | ||
}; | ||
|
||
new AutosuggestDropdownController( this._input, { | ||
|
||
list: explanationList, | ||
|
||
header: 'Narrow your search', | ||
|
||
classNames: { | ||
container: 'search-bar__dropdown-menu-container', | ||
header: 'search-bar__dropdown-menu-header', | ||
list: 'search-bar__dropdown-menu', | ||
item: 'search-bar__dropdown-menu-item', | ||
activeItem: 'js-search-bar-dropdown-menu-item--active', | ||
}, | ||
|
||
renderListItem: (listItem)=>{ | ||
|
||
let itemContents = `<span class="search-bar__dropdown-menu-title"> ${listItem.title} </span>`; | ||
|
||
if (listItem.explanation){ | ||
itemContents += `<span class="search-bar__dropdown-menu-explanation"> ${listItem.explanation} </span>`; | ||
} | ||
|
||
return itemContents; | ||
}, | ||
|
||
listFilter: function(list, currentInput){ | ||
|
||
currentInput = (currentInput || '').trim(); | ||
|
||
return list.filter((item)=>{ | ||
|
||
if (!currentInput){ | ||
return item; | ||
} else if (currentInput.indexOf(':') > -1) { | ||
return false; | ||
} | ||
return item.title.toLowerCase().indexOf(currentInput) >= 0; | ||
|
||
}).sort((a,b)=>{ | ||
|
||
// this sort functions intention is to | ||
// sort partial matches as lower index match | ||
// value first. Then let natural sort of the | ||
// original list take effect if they have equal | ||
// index values or there is no current input value | ||
|
||
if (!currentInput){ | ||
return 0; | ||
} | ||
|
||
let aIndex = a.title.indexOf(currentInput); | ||
let bIndex = b.title.indexOf(currentInput); | ||
|
||
if (aIndex > bIndex){ | ||
return 1; | ||
} else if (aIndex < bIndex){ | ||
return -1; | ||
} | ||
return 0; | ||
}); | ||
}, | ||
|
||
onSelect: (itemSelected)=>{ | ||
selectFacet(itemSelected.title); | ||
}, | ||
|
||
}); | ||
|
||
|
||
/** | ||
* Insert a hidden <input> with an empty value into the search <form>. | ||
* | ||
|
@@ -207,12 +118,184 @@ class SearchBarController extends Controller { | |
} | ||
}; | ||
|
||
|
||
this._hiddenInput = insertHiddenInput(this.refs.searchBarForm); | ||
|
||
let explanationList = [ | ||
{ | ||
matchOn: 'user', | ||
title: 'user:', | ||
explanation: 'search by username', | ||
}, | ||
{ | ||
matchOn: 'tag', | ||
title: 'tag:', | ||
explanation: 'search for annotations with a tag', | ||
}, | ||
{ | ||
matchOn: 'url', | ||
title: 'url:', | ||
explanation: 'see all annotations on a page', | ||
}, | ||
{ | ||
matchOn: 'group', | ||
title: 'group:', | ||
explanation: 'show annotations created in a group you are a member of', | ||
}, | ||
].map((item)=>{ return Object.assign(item, { type: FACET_TYPE}); }); | ||
|
||
// tagSuggestions are made available by the scoped template data. | ||
// see search.html.jinja2 for definition | ||
const tagSuggestionJSON = document.querySelector('.js-tag-suggestions'); | ||
let tagSuggestions = []; | ||
|
||
if(tagSuggestionJSON){ | ||
try{ | ||
tagSuggestions = JSON.parse(tagSuggestionJSON.innerHTML.trim()); | ||
}catch(e){ | ||
console.error('Could not parse .js-tag-suggestions JSON content', e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If finding the tag succeeds but parsing the JSON fails this will lead to some confusing errors below because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just changed the logic to not reuse a variable. I don't like doing that for this reason- didn't think about it while jumping around though. |
||
} | ||
} | ||
|
||
let tagsList = ((tagSuggestions) || []).map((item)=>{ | ||
return Object.assign(item, { | ||
type: TAG_TYPE, | ||
title: escapeHtml(item.tag), // make safe | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine, although I'm concerned that there will be a mixup with escaping in future, where something escaped for HTML is incorrectly used in a different context or some transformation gets applied to the string which breaks the escaping (on the Python side, this hazard is avoided by wrapping escaped strings using a special |
||
matchOn: stringUtil.fold(stringUtil.normalize(item.tag)), | ||
usageCount: item.count || 0, | ||
}); | ||
}); | ||
|
||
|
||
this._suggestionsHandler = new AutosuggestDropdownController( this._input, { | ||
|
||
list: explanationList.concat(tagsList), | ||
|
||
header: 'Narrow your search', | ||
|
||
classNames: { | ||
container: 'search-bar__dropdown-menu-container', | ||
header: 'search-bar__dropdown-menu-header', | ||
list: 'search-bar__dropdown-menu', | ||
item: 'search-bar__dropdown-menu-item', | ||
activeItem: 'js-search-bar-dropdown-menu-item--active', | ||
}, | ||
|
||
renderListItem: (listItem)=>{ | ||
|
||
let itemContents = `<span class="search-bar__dropdown-menu-title"> ${listItem.title} </span>`; | ||
|
||
if (listItem.explanation){ | ||
itemContents += `<span class="search-bar__dropdown-menu-explanation"> ${listItem.explanation} </span>`; | ||
} | ||
|
||
return itemContents; | ||
}, | ||
|
||
listFilter: (list, currentInput)=>{ | ||
|
||
currentInput = (currentInput || '').trim(); | ||
|
||
let typeFilter = currentInput.indexOf('tag:') === 0 ? TAG_TYPE : FACET_TYPE; | ||
let inputFilter = stringUtil.fold(stringUtil.normalize(currentInput)); | ||
|
||
if(typeFilter === TAG_TYPE){ | ||
inputFilter = inputFilter.substr(/*'tag:' len*/4); | ||
|
||
// remove the initial quote for comparisons if it exists | ||
if(inputFilter[0] === '\'' || inputFilter[0] === '"'){ | ||
inputFilter = inputFilter.substr(1); | ||
} | ||
} | ||
|
||
if(this.state.suggestionsType !== typeFilter){ | ||
this.setState({ | ||
suggestionsType: typeFilter, | ||
}); | ||
} | ||
|
||
return list.filter((item)=>{ | ||
return item.type === typeFilter && item.matchOn.toLowerCase().indexOf(inputFilter.toLowerCase()) >= 0; | ||
}).sort((a,b)=>{ | ||
|
||
// this sort functions intention is to | ||
// sort partial matches as lower index match | ||
// value first. Then let natural sort of the | ||
// original list take effect if they have equal | ||
// index values or there is no current input value | ||
|
||
if (inputFilter){ | ||
let aIndex = a.matchOn.indexOf(inputFilter); | ||
let bIndex = b.matchOn.indexOf(inputFilter); | ||
|
||
// match score | ||
if (aIndex > bIndex){ | ||
return 1; | ||
} else if (aIndex < bIndex){ | ||
return -1; | ||
} | ||
} | ||
|
||
|
||
// If we are filtering on tags, we need to arrange | ||
// by popularity | ||
if(typeFilter === TAG_TYPE){ | ||
if(a.usageCount > b.usageCount){ | ||
return -1; | ||
}else if(a.usageCount < b.usageCount) { | ||
return 1; | ||
} | ||
} | ||
|
||
return 0; | ||
|
||
}).slice(0, MAX_SUGGESTIONS); | ||
}, | ||
|
||
onSelect: (itemSelected)=>{ | ||
|
||
if (itemSelected.type === TAG_TYPE){ | ||
let tagSelection = itemSelected.title; | ||
|
||
// wrap multi word phrases with quotes to keep | ||
// autosuggestions consistent with what user needs to do | ||
if(tagSelection.indexOf(' ') > -1){ | ||
tagSelection = `"${tagSelection}"`; | ||
} | ||
|
||
addLozenge('tag:' + tagSelection); | ||
this._input.value = ''; | ||
} else { | ||
this._input.value = itemSelected.title; | ||
setTimeout(()=>{ | ||
this._input.focus(); | ||
}, 0); | ||
} | ||
updateHiddenInput(); | ||
}, | ||
|
||
}); | ||
|
||
this._input.addEventListener('keydown', onInputKeyDown); | ||
this._input.addEventListener('input', updateHiddenInput); | ||
lozengifyInput(); | ||
} | ||
|
||
update(newState, prevState){ | ||
|
||
if(!this._suggestionsHandler){ | ||
return; | ||
} | ||
|
||
if(newState.suggestionsType !== prevState.suggestionsType){ | ||
if(newState.suggestionsType === TAG_TYPE){ | ||
this._suggestionsHandler.setHeader('Popular tags'); | ||
}else { | ||
this._suggestionsHandler.setHeader('Narrow your search'); | ||
} | ||
} | ||
|
||
} | ||
} | ||
|
||
module.exports = SearchBarController; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to just rename
_setHeader
tosetHeader
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to not mix internals and public api. Or rather, make it super obvious which is which