Skip to content

Limit autocomplete to keyword searches#925

Draft
marlo-longley wants to merge 1 commit intomainfrom
hide-autocomplete
Draft

Limit autocomplete to keyword searches#925
marlo-longley wants to merge 1 commit intomainfrom
hide-autocomplete

Conversation

@marlo-longley
Copy link
Contributor

Closes #864

@marlo-longley marlo-longley marked this pull request as ready for review April 2, 2025 21:18
@@ -0,0 +1,22 @@
export default function initializeAutocompleteToggle() {
const searchField = document.querySelector('#search_field');
Copy link
Contributor

@jcoyne jcoyne Apr 3, 2025

Choose a reason for hiding this comment

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

Is there a reason you aren't using Stimulus for this? All of the other JS for this app uses Stimulus.

Copy link
Contributor Author

@marlo-longley marlo-longley Apr 3, 2025

Choose a reason for hiding this comment

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

I wrote this in Stimulus first. I needed to copy down the Blacklight template for the search bar in order to insert the Stimulus targets, controller, etc. into the markup. I got the feedback that this seemed messier than writing vanilla JS. With vanilla JS, there was no need to copy down the Blacklight component. So I rewrote it this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

By making that choice, you are binding to selectors that are not guaranteed to be stable between versions of blacklight/arclight. This seems more dangerous to me, even though it might be less code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes sense, thanks for the feedback. The team decided put this to draft for now as we learn more about what the stakeholders actually want from this feature.

@marlo-longley marlo-longley marked this pull request as draft April 3, 2025 17:24
@marlo-longley
Copy link
Contributor Author

@corylown should we close this and the linked ticket in light of #1008 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

typeahead options are confusing to users

3 participants