Query Suggestions #27
Conversation
bblaisATcoveo
had a problem deploying
to
github-ci
GitHub Actions
Failure
There was a problem hiding this comment.
@bblaisATcoveo Thank you for your contribution. Please see requested changes.
src/connector.js
Outdated
| @@ -31,11 +31,13 @@ const defaults = { | |||
| "searchBoxQuery": "#sch-inp-ac", | |||
| "lang": "en", | |||
| "numberOfSuggestions": 0, | |||
| "minimumCharsForSuggestions": 1, | |||
| "unsupportedSuggestions": false, | |||
There was a problem hiding this comment.
This parameter ca be removed
| "unsupportedSuggestions": false, |
src/connector.js
Outdated
| "unsupportedSuggestions": false, | ||
| "enableHistoryPush": true, | ||
| "isContextSearch": false, | ||
| "isAdvancedSearch": false, | ||
| "originLevel3": window.location.origin + winPath | ||
| "originLevel3": window.location.origin + winPath, | ||
| "pipeline": "" |
There was a problem hiding this comment.
Can you please document briefly the purpose of this variable in your PR description? Or keep it for a separate PR since I believe it is unrelated to QS? Thank you.
src/connector.js
Outdated
| @@ -69,6 +71,13 @@ let querySummaryState; | |||
| let didYouMeanState; | |||
| let pagerState; | |||
| let lastCharKeyUp; | |||
| let activeSuggestion = 0; | |||
| let activeSuggestionWaitMouseMove = true; | |||
| let roughExperiment = false; | |||
There was a problem hiding this comment.
This variable can be removed, as well as everything that it causes (in the if statement), since we are essentially making the QS stable through this PR.
There was a problem hiding this comment.
@GormFrank I thought it might be useful to keep the rough experiment flag in case we need it for another feature
src/connector.js
Outdated
| `<p class="h5">Rechercher plutôt <button class="btn btn-lg btn-link p-0 mrgn-bttm-sm" type="button">%[correctedQuery]</button> ?</p>`; | ||
| `<p class="h5 mrgn-lft-md">Rechercher plutôt <button class="btn btn-lg btn-link p-0" type="button">%[correctedQuery]</button> ?</p>`; | ||
| } | ||
| else { | ||
| didYouMeanTemplateHTML = | ||
| `<p class="h5">Did you mean <button class="btn btn-lg btn-link p-0 mrgn-bttm-sm" type="button">%[correctedQuery]</button> ?</p>`; | ||
| `<p class="h5 mrgn-lft-md">Did you mean <button class="btn btn-lg btn-link p-0" type="button">%[correctedQuery]</button> ?</p>`; |
There was a problem hiding this comment.
I don't believe your PR should be affecting the template for Did you mean
src/connector.js
Outdated
| `<h2>%[numberOfResults] résultats de recherche pour "%[query]"</h2>`; | ||
| `%[numberOfResults] résultats de recherche pour "%[query]"`; | ||
| } | ||
| else { | ||
| querySummaryTemplateHTML = | ||
| `<h2>%[numberOfResults] search results for "%[query]"</h2>`; | ||
| `%[numberOfResults] search results for "%[query]"`; |
There was a problem hiding this comment.
I don't believe your PR should be affecting the template for Query Summary
src/connector.js
Outdated
| if( params.isContextSearch ) { | ||
| author = author.replace( ';', ', ' ); | ||
| } | ||
| else { | ||
| author = author.replace( ',', ';' ); | ||
| author = author.replace( ';' , '</li> <li>' ); | ||
| } | ||
|
|
||
| author = author.replaceAll( ';' , '</li> <li>' ); |
There was a problem hiding this comment.
Can you please document this change in the description of your PR? Or open a separate PR with this change, since it is most likely unrelated to QS.
src/connector.js
Outdated
| let breadcrumb = ""; | ||
| if ( result.raw.hostname && result.raw.displaynavlabel ) { | ||
| const splittedNavLabel = ( Array.isArray( result.raw.displaynavlabel ) ? result.raw.displaynavlabel[0] : result.raw.displaynavlabel).split( '>' ); | ||
| breadcrumb = result.raw.hostname + ' </li><li>' + splittedNavLabel[splittedNavLabel.length-1]; |
There was a problem hiding this comment.
Can you please document this change in the description of your PR? Or open a separate PR with this change, since it is most likely unrelated to QS.
src/connector.js
Outdated
| // Create the <h2> element | ||
| const hTwoAnchor = document.createElement("h2"); | ||
| // Generate the text content | ||
| const querySummaryHTML = ( ( querySummaryState.query !== "" && !params.isAdvancedSearch ) ? querySummaryTemplateHTML : noQuerySummaryTemplateHTML ) | ||
| .replace( '%[numberOfResults]', numberOfResults ) | ||
| .replace( '%[query]', '<span class="sr-query"></span>' ) | ||
| .replace( '%[queryDurationInSeconds]', querySummaryState.durationInSeconds.toLocaleString( params.lang ) ); | ||
|
|
||
| querySummaryElement.innerHTML = querySummaryHTML; | ||
|
|
||
| const queryElement = querySummaryElement.querySelector( '.sr-query' ); | ||
| if ( queryElement ){ | ||
| queryElement.textContent = querySummaryState.query; | ||
| } | ||
| const querySummaryText = ((querySummaryState.query !== "" && !params.isAdvancedSearch) ? querySummaryTemplateHTML : noQuerySummaryTemplateHTML) | ||
| .replace('%[numberOfResults]', numberOfResults) | ||
| .replace('%[query]', querySummaryState.query) | ||
| .replace('%[queryDurationInSeconds]', querySummaryState.durationInSeconds.toLocaleString(params.lang)); | ||
| hTwoAnchor.textContent = querySummaryText; | ||
| querySummaryElement.innerHTML = ""; | ||
| querySummaryElement.appendChild(hTwoAnchor); |
There was a problem hiding this comment.
I don't believe your PR should be affecting this section; maybe you didn't do a rebase on the upstream?
src/connector.js
Outdated
| @@ -1033,4 +1146,4 @@ function updatePagerState( newState ) { | |||
| } | |||
|
|
|||
| // Run Search UI | |||
| initSearchUI(); | |||
| initSearchUI(); | |||
There was a problem hiding this comment.
Please add an empty new line at the end of your file :)
src/connector.js
Outdated
| // Arrow key up | ||
| else if ( e.keyCode === 38 ) { | ||
| if ( !( isFirefox && waitForkeyUp ) ){ | ||
| waitForkeyUp = true; | ||
| searchBoxArrowKeyUp(); | ||
| e.preventDefault(); | ||
| } | ||
| } | ||
| // Arrow key down | ||
| else if ( e.keyCode === 40 ) { | ||
| if ( !( isFirefox && waitForkeyUp ) ){ | ||
| waitForkeyUp = true; | ||
| searchBoxArrowKeyDown(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Since the isFirefox variable is used only here, and seems to be avoiding entering in the if, I'm asking myself why doesn't it need to enter the if? Isn't there a way to have a universal approach to this instead?
Enable Query Suggestions and fix various issues.