Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Handle quotation marks in search strings and adjust query result header styling #770

Merged
merged 3 commits into from
Aug 25, 2023
Merged

Handle quotation marks in search strings and adjust query result header styling #770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44c7630
Handle unclosed quotations in search string
dchiller Aug 24, 2023
f88915b
Change display style for query in search result header
dchiller Aug 24, 2023
99ee8af
Modify display of queried string for volpiano and mode searches
dchiller Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions nginx/public/node/frontend/public/css/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,3 +679,8 @@ html.js {
word-wrap: break-word;
}
}

.displayed-query{
color: #c2342e;
font-weight: bold;
}
13 changes: 12 additions & 1 deletion nginx/public/node/frontend/public/js/app/search/chant-search/ChantSearchProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,18 @@ export default Marionette.Object.extend({
// Modify query returned by solr so it is ready for display.
// Remove escaping backslashes from the displayed string.
let query = this.searchParameters.get('query');
let displayedQuery = (fieldName === "volpiano" || fieldName === "volpiano_literal") ? query.replaceAll("\\-", "-") : query;
let displayedQuery;
switch (fieldName){
case "volpiano":
case "volpiano_literal":
displayedQuery = query.replaceAll("\\-", "-");
break;
case "mode":
displayedQuery = query.join(", ");
break;
default:
displayedQuery = query;
}
return {
fieldName: fieldName,
query: query,
Expand Down
6 changes: 6 additions & 0 deletions nginx/public/node/frontend/public/js/app/search/chant-search/SearchInputView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ export default Marionette.ItemView.extend({
// escaped hyphens in the query string.
searchInput = searchInput.replaceAll("-","\\-");
}
// Handle quotations in text field searches. Solr errors if quotation marks
// are not closed. If the search string contains an odd number of quotation
// marks, add a quotation mark to the end of the string.
if (["all","feast","genre","office"].includes(searchField)){
(searchInput.split('"').length - 1) % 2 === 1 ? searchInput += '"' : null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this handle single quotation marks too, and should we be concerned about quotations within quotations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just handles double marks...

The honest reason is that solr does not complain with single marks (or many other punctuation marks that you wouldn't expect in any of our text -- eg. # @ % & etc). As far as I can tell, solr just strips out the single marks.

Solr does respond to other reserved characters when entered in the search box (eg. - and () and I do think we need a better way of making this search syntax available to the user.

In any case, single marks don't cause the error so I didn't fix them here, but it's a good point that ideally we need to figure out what's going on in these other cases.

}
// FIXME(wabain): While this class needs to take a SearchInput model so it can initially
// be rendered, we're not actually updating that model here - we're just triggering
// an event which will cause the appropriate changes to propagate. That's kind of
Expand Down
6 changes: 3 additions & 3 deletions nginx/public/node/frontend/public/js/app/search/search-result-heading.template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<p class="h4"><i><%- numFound %> <%- numFound === 1 ? 'result' : 'results' %> for query
<p class="h5"><i><%- numFound %> <%- numFound === 1 ? 'result' : 'results' %> for query:</i>
<% if (fieldName == "volpiano" || fieldName == "volpiano_literal") { %>
</i><span class = "volpiano">1-<%- displayedQuery %></span>
<span class = "volpiano">1-<%- displayedQuery %></span>
<% } else { %>
"<%- query %>"</i>
<span class="displayed-query"><%- displayedQuery %></span>
<% } %>
</p>