Skip to content

Commit

Permalink
Fixed debounced search overriding url-match results when searching links
Browse files Browse the repository at this point in the history
no issue

When the query matches a URL we immediately call the `search()` method to replace the results with an "Insert URL" result but if a debounced search was still running that could then overwrite the "Insert URL" result when it finishes debouncing/running.

- added an explicit debounce cancel when we make an immediate `search()` call
- updated tests to reduce flakiness by ensuring the spinner is not visible before comparing html
  • Loading branch information
kevinansfield committed Jun 12, 2024
1 parent 4ac1dd9 commit 96a6a9a
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export function InputListGroup({dataTestId, group, showSpinner}) {
<li className="mb-0 mt-2 flex items-center justify-between border-t border-grey-200 px-4 pb-2 pt-3 text-[1.1rem] font-semibold uppercase tracking-wide text-grey-600 first-of-type:mt-0 first-of-type:border-t-0 dark:border-grey-900" data-testid={`${dataTestId}-listGroup`}>
<div className="flex items-center gap-1.5">
{group.label}
{showSpinner && <span className="ml-px"><Spinner size="mini" /></span>}
{showSpinner && <span className="ml-px" data-testid="input-list-spinner"><Spinner size="mini" /></span>}
</div>
</li>
);
Expand Down
1 change: 1 addition & 0 deletions packages/koenig-lexical/src/hooks/useSearchLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export const useSearchLinks = (query, searchLinks, {noResultOptions} = {}) => {
// perform a non-debounced search if the query is a URL so the
// "Link to web page" option updates more responsively
if (URL_QUERY_REGEX.test(query)) {
debouncedSearch.cancel();
search(query);
} else {
debouncedSearch(query);
Expand Down
4 changes: 4 additions & 0 deletions packages/koenig-lexical/test/e2e/internal-linking.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ test.describe('Internal linking', async () => {
await expect(page.getByTestId('bookmark-url-dropdown')).toBeVisible();

await page.keyboard.type('http');
await expect(page.getByTestId('input-list-spinner')).not.toBeVisible();

await assertHTML(page, html`
<li><div>Link to web page</div></li>
Expand All @@ -133,6 +134,7 @@ test.describe('Internal linking', async () => {
await expect(page.getByTestId('bookmark-url-dropdown')).toBeVisible();

await page.keyboard.type('#test');
await expect(page.getByTestId('input-list-spinner')).not.toBeVisible();

await assertHTML(page, html`
<li><div>Link to web page</div></li>
Expand All @@ -151,6 +153,7 @@ test.describe('Internal linking', async () => {
await expect(page.getByTestId('bookmark-url-dropdown')).toBeVisible();

await page.keyboard.type('/test');
await expect(page.getByTestId('input-list-spinner')).not.toBeVisible();

await assertHTML(page, html`
<li><div>Link to web page</div></li>
Expand All @@ -169,6 +172,7 @@ test.describe('Internal linking', async () => {
await expect(page.getByTestId('bookmark-url-dropdown')).toBeVisible();

await page.keyboard.type('mailto:test@example.com');
await expect(page.getByTestId('input-list-spinner')).not.toBeVisible();

await assertHTML(page, html`
<li><div>Link to web page</div></li>
Expand Down

0 comments on commit 96a6a9a

Please sign in to comment.