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

Highlight (goTo) the first item on each keystroke #241

Closed
mileusna opened this issue Jun 8, 2021 · 10 comments
Closed

Highlight (goTo) the first item on each keystroke #241

mileusna opened this issue Jun 8, 2021 · 10 comments
Labels
enhancement New feature or request

Comments

@mileusna
Copy link

mileusna commented Jun 8, 2021

How can I highlight (goTo) to the first item in the results on each keystroke?

I tried with this, but this will work only on the first keystroke, when result box is opened.

document.querySelector("#autoselect").addEventListener("open", function (event) {
    autoSelect.goTo(0);
});    

Is there an event which will fire on earch refresh of the results?

@mileusna mileusna added the enhancement New feature or request label Jun 8, 2021
@TarekRaafat
Copy link
Owner

Hello @mileusna,

You can use the results event instead of open and it should give you the results you're looking for.

Please try it and let me know how it goes.

@mileusna
Copy link
Author

mileusna commented Jun 8, 2021

@TarekRaafat thank you for quick response

I have tried that already but it looks like the results event is fired when the results are ready, not after they are displayed, so it doesn't work out of the box.

However, I have managed to achieve what I want with small delay in results event using setTimeout, but I would appreciate some straight forward solution, if there is any.

document.querySelector("#autoselect").addEventListener("results", function (event) {
   setTimeout(() => {  autoSelect.goTo(0); }, 100);
});  

@folknor
Copy link

folknor commented Jun 8, 2021

You should be able to use config.resultsList.element. It's invoked by autoComplete.js every time the list is updated, right before it's opened.

I've not tested it, but seems to me it should work.

const autoSelect = new autoComplete({
	data: {
		src: ...,
	},
	resultsList: {
		element: (element, data) => {
			//const { input, query, matches, results } = data
			autoSelect.goTo(0)
		}
	},
})

@TarekRaafat
Copy link
Owner

@mileusna, you're actually right using the results event would require a delay because it comes before the list opens.

I have tried the code below as you mentioned and it worked as expected.

events: {
  input: {
    results: () => setTimeout(() => autoCompleteJS.goTo(0), 5),
  }
}

I'll look into it and get back to you if I find a more straightforward way of achieving it.

@folknor same goes for your solution, it would require a delay because resultsList.element precedes the list opening.

resultsList: {
  element: (list, data) => {
    setTimeout(() => autoCompleteJS.goTo(0), 5);
  }
}

@folknor
Copy link

folknor commented Jun 9, 2021

@folknor same goes for your solution, it would require a delay because resultsList.element precedes the list opening.

Are you certain?

The only thing open does is it removes the hidden attribute, right? All the HTML has been created when .element is called.

As far as I can see, all the actions in goTo are valid as long as the HTML has been created, and all the changes made in goTo should still be present after the hidden attribute is removed from the container.

If you are correct, you might want to test using styles like display:none and/or visibility:hidden instead of the hidden attribute.

@folknor
Copy link

folknor commented Jun 10, 2021

I just tested it and my solution works if you change this line: https://github.com/TarekRaafat/autoComplete.js/blob/master/src/controllers/listController.js#L121 by removing the ctx.isOpen condition, so the line looks like if (results.length) {.

Without that change, my solution works for every new search except the first time the list opens.

@folknor
Copy link

folknor commented Jun 27, 2021

@TarekRaafat In case my previous comments were unclear; the code I posted in #241 (comment) works perfectly if you remove the condition like I said in #241 (comment)

@TarekRaafat
Copy link
Owner

Hey @folknor,

Apologies for my delayed reply.

Thanks for the clarification. I get your point.

Unfortunately, there's a downside for removing the above-mentioned condition: you're going to get an error if you used the goTo(1) API while the list is closed, and there are no results, as shown below.

Screen Shot 2021-06-28 at 9 53 29 AM

Screen Shot 2021-06-28 at 9 54 04 AM

Let alone that I couldn't think of any use case that might require using the goTo(1) API while the list is closed, and please let me know if there are any.

Accordingly, using a set timeout in both ways mentioned above looks more convenient for the time being until a better idea comes up.

Please, let me know your thoughts.

Cheers, and have a nice day! :)

@folknor
Copy link

folknor commented Jun 28, 2021

Take all the time you want, I was not concerned about any delay. I was concerned that maybe I was communicating my thoughts poorly.

I think there is something here that is misunderstood. I will try to express it very clearly :-)

  1. The condition could change from if (ctx.isOpen && results.length) { to if (results.length) {, then it should never error like your screenshot
  2. The code in my first comment used goTo(0), not goTo(1), but it will not error whatever value you pass to goTo in my tests

To me this seems cleaner. It avoids lots of setTimeout calls, and seems more convenient.

I've tested it with noResults: true and false, with goTo values of 1-10, and with different kinds of autoComplete.js data sources. I can not find any problem with the solution in my first comment, if you change the condition in goTo in the autoComplete.js code.

There is either some problem I can't see, or I am not making myself understood.

Thank you!

@TarekRaafat
Copy link
Owner

My bad, I missed mentioning the point where exactly the error happens.

The error actually occurs after using the goTo(x) and then using the select() afterward on an empty results list or before list rendering.

Due to the cursor location change at the goTo(x) stage, the select() verifies that the cursor has moved. Hence, it performs the selection process to get the pointed result item that already does not exists, and that's where it generates an error.

I understand that it could be a rare case to happen, yet I believe safety is needed.

Please let me know your thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants