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.

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

feat: Add query parameter to custom suggestionTemplate function #1516

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

andygout
Copy link
Contributor

@andygout andygout commented Mar 27, 2024

Describe your changes

This PR makes changes which extend on those introduced by #1127 and enables highlighting of custom rendering of suggestions.

If the user does not provide a suggestionTemplate in the options when instantiating an instance of oAutocomplete then the Autocomplete.suggestionTemplate will be used:

/**
* Used when rendering suggestions, the return value of this will be used as the innerHTML for a single suggestion.
*
* @param {string} suggestedValue The suggestion to apply the template with.
* @returns {string} HTML string to be represent a single suggestion.
*/
suggestionTemplate (suggestedValue) {
// o-autocomplete has a UI design to highlight characters in the suggestions.
const input = this.autocompleteEl.querySelector('input');
/**
* @type {CharacterHighlight[]} An array of arrays which contain two items, the first is the character in the suggestion, the second is a boolean which indicates whether the character should be highlighted.
*/
const characters = highlightSuggestion(suggestedValue, input ? input.value : suggestedValue);
let output = '';
for (const [character, shoudHighlight] of characters) {
if (shoudHighlight) {
output += `<span class="o-autocomplete__option--highlight">${character}</span>`;
} else {
output += `${character}`;
}
}
const span = document.createElement('span');
span.setAttribute('aria-label', suggestedValue);
span.innerHTML = output;
return span.outerHTML;
}

This has a direct reference to autocompleteEl (via this.autocompleteEl), which can be used to acquire the current text input value, which is then used to enable highlighting of the suggestion.

However, when providing a custom suggestionTemplate function, this direct reference to autocompleteEl is not an option. It is possible to obtain the text input via const input = document.getElementById('autocomplete-input');, though to do this prior to rendering every suggestion is inefficient.

The changes made in PR Financial-Times/accessible-autocomplete#29 means that query (the text which was typed into the autocomplete text input field by the user) can be included in a custom suggestionTemplate callback function's parameters and so used to highlight the option value (or, in the scenario of option being an object, a value thereof).

This screengrab demonstrates the benefit of these changes via a scenario in which a custom suggestionTemplate function was used (in order to include the suffix that describes the type of thing the suggestion is: person, venue, company, etc.) and where the suggestion's name is also highlighted in accordance with the text the user has input:

autocomplete-custom-highlighted-suggestion

Open question: Autocomplete.suggestionTemplate can continue to acquire the query value via const input = this.autocompleteEl.querySelector('input'); (and then acquire the value from input.value); to remove that line and provide the query in the same way as a custom suggestionTemplate function (as a second function parameter) results in linting error (see here): Error: 396:21 error Expected 'this' to be used by class method 'suggestionTemplate' class-methods-use-this. This implies the class method would need to be changed into a standalone function which in turn entails a fair amount of overhauling perhaps excessive for the intention of this work, though I'd be interested to hear opinions.

I've added @adgad as a reviewer on the basis that they may be familiar with this code from their work on #1127.

Issue ticket number and link

N/A

Link to Figma designs

N/A

TODO (pre-merge)

  • Upgrade @financial-times/accessible-autocomplete version to that which includes changes included in PR Add query arg to suggestionTemplate function accessible-autocomplete#29
  • Identify if the Storybook demos for dynamic-custom-highlighted-suggestion (and dynamic-custom-suggestion) should have their hidden value set to true
    • Update: The intention is to keep things in Storybook as what people get from Origami out of the box, so the demos whose hidden value is set to true should remain as such

Dependency PRs

Release

This adds functionality in a backwards-compatible manner so I propose to release these changes as a minor version.

Checklist before requesting a review

  • I have applied percy label on my PR before merging and after review.
  • If it is a new feature, I have added thorough tests.
  • I have updated relevant docs.
  • I have updated relevant env variables in Doppler. — N/A

@andygout andygout requested a review from adgad March 27, 2024 22:09
@andygout andygout force-pushed the feat-add-query-parameter-to-suggestion-template-function branch 4 times, most recently from 764b9a9 to 5186703 Compare March 27, 2024 22:47
@andygout andygout changed the title feat: Add query parameter to suggestionTemplate function feat: Add query parameter to custom suggestionTemplate function Mar 27, 2024
@notlee notlee temporarily deployed to origami-webs-feat-add-q-gwv2go March 27, 2024 22:52 Inactive
@andygout andygout force-pushed the feat-add-query-parameter-to-suggestion-template-function branch from 5186703 to 3d0cd4c Compare March 29, 2024 18:42
@notlee notlee temporarily deployed to origami-webs-feat-add-q-gwv2go March 29, 2024 18:42 Inactive
@andygout andygout force-pushed the feat-add-query-parameter-to-suggestion-template-function branch from 3d0cd4c to 28bee2b Compare March 30, 2024 11:25
@notlee notlee temporarily deployed to origami-webs-feat-add-q-gwv2go March 30, 2024 11:25 Inactive
@andygout andygout force-pushed the feat-add-query-parameter-to-suggestion-template-function branch from 28bee2b to c21ca85 Compare April 18, 2024 16:45
@notlee notlee temporarily deployed to origami-webs-feat-add-q-auylra April 18, 2024 16:50 Inactive
@notlee notlee temporarily deployed to origami-webs-feat-add-q-auylra April 18, 2024 16:52 Inactive
@andygout andygout force-pushed the feat-add-query-parameter-to-suggestion-template-function branch from 1b8de6b to a9d52cb Compare April 18, 2024 16:54
@notlee notlee temporarily deployed to origami-webs-feat-add-q-auylra April 18, 2024 16:55 Inactive
@andygout andygout marked this pull request as ready for review April 19, 2024 08:12
@andygout andygout requested a review from a team as a code owner April 19, 2024 08:12
Copy link
Contributor

@notlee notlee left a comment

Choose a reason for hiding this comment

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

:shipit:

@notlee notlee merged commit c326099 into main Apr 19, 2024
9 checks passed
@notlee notlee deleted the feat-add-query-parameter-to-suggestion-template-function branch April 19, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants