-
Notifications
You must be signed in to change notification settings - Fork 1.3k
search: accommodate for fast typers #2031
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This can be potentially crucial when trying to debug failures of CI builds that are caused by diverging Pagefind versions. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When trying to enhance the test suite, it can be a bit tricky to figure out what sparse-checkout definition to use. The alternative would be to build the entire site, which would be prohibitively slow for an efficient development cycle. Therefore, let's document that (quite involved, I am afraid!) sparse checkout definition for the minimum set of files required to build the site and run the test suite. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The highly recommended paved path for developing this site on Windows is to use the Windows Subsystem for Linux. The reason for that is that Windows' filename restrictions prevent Hugo from accommodating the URLs containing URL-encoded question marks that git-scm.com has for historical reasons. However, I understand that there _are_ circumstances under which it might be necessary to develop this site in pure Windows, without the help of Linux. Let's describe what it takes to do that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
On non-Linux platforms, the prerequisite screenshots will not exist before the first run of that test case, which will save them (so that subsequent runs will succeed). This quirk might surprise developers on platforms other than Linux, therefore it is a good idea to document it. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
On Windows, file names cannot contain question marks. We describe a work-around to allow rendering the site (which does contain pages whose URLs contain literal question marks, for historical reasons, and that can therefore unfortunately not be changed) by changing the affected `url` front-matter attributes to _not_ contain question marks. To let the resulting site pass the test suite, we need to adjust it a little. We also need to give the test more time because the `manual pages` test case seems to take a bit longer than those 30 seconds which are the default timeout of Playwright, at least in my setup. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When the search failed fatally for some reason, returning `null` instead of an empty-sized array of results, let's still tell the person viewing the search results that none could be found. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When a person types really fast into the search box and also hits the Enter key really quickly after that, the current code directs the browser to https://git-scm.com/undefined. The reason? At that stage, the search results had not yet been retrieved and as a consequence, the list of results to be shown in the live search were not populated. But that's exactly from where this URL was obtained. Let's just initialize that URL a lot earlier. This fixes git#2030 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When someone currently presses the `/` key to let the search box get the focus, and then immediately presses the Enter key without typing a search term, the site currently redirects to https://git-scm.com/undefined. The same happens when the search term consists of only one character. That's because it is somewhat unexpected that anybody would want to see _all_ search results before seeing _some_ search results. Let's fix that. Incidentally, this also helps with adding a test that verifies that hitting Enter in the search box will never direct the browser to that `/undefined` URL, no matter how quickly that search term and the Enter key are provided: The `runSearch()` function triggers on key-up, which Playwright's `fill()` method (which can be used to fill in a search term) does not emit. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We just fixed a bug where the search results URL wasn't yet generated, and as a result the `https://git-scm.com/undefined` URL was used when a user typed a search term really fast and hit Enter. Let's make sure that this bug never regresses. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
For convenience, I deployed this to https://dscho.github.io/git-scm.com/. |
To1ne
reviewed
Jul 31, 2025
@dscho Code looks good, seems to fix the bug, but I've noticed when you're really fast only part of the search string is used. I guess this is just because of some slowness in javascript? Using a proper I approve. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Context
This fixes #2030