Skip to content

Commit d5f591e

Browse files
fix(lxl-web): Fix failing tests due to diffing value and supersearch changes (#1572)
* Fix comparison if values with wrapping parens * Remove unused params * Add workaround fixing back/forward navigation when submitting and navigating from expanded dialog
1 parent a7862d8 commit d5f591e

7 files changed

Lines changed: 73 additions & 73 deletions

File tree

‎lxl-web/src/lib/components/supersearch/SuperSearchWrapper.svelte‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import IconSearch from '~icons/bi/search';
2727
import '$lib/styles/lxlquery.css';
2828
import { getSearchContext } from '$lib/contexts/search';
29+
import { SvelteURLSearchParams } from 'svelte/reactivity';
2930
3031
interface Props {
3132
placeholder: string;
@@ -314,10 +315,6 @@
314315
});
315316
}
316317
317-
function submit(form: HTMLFormElement) {
318-
superSearch?.submit(form);
319-
}
320-
321318
function addQualifierKey(qualifierKey: string) {
322319
superSearch?.resetData();
323320
showExpandedSearch(); // keep dialog open (since 'regular' search is hidden on mobile)
@@ -444,7 +441,6 @@
444441
searchContext.showExpandedSearch = showExpandedSearch;
445442
searchContext.hideExpandedSearch = hideExpandedSearch;
446443
searchContext.changeQuery = changeQuery;
447-
searchContext.submit = submit;
448444
searchContext.isMounted = true;
449445
});
450446
@@ -473,13 +469,15 @@
473469
{autofocus}
474470
endpoint={`/api/${page.data.locale}/supersearch`}
475471
queryFn={(query, cursor) => {
476-
return new URLSearchParams({
472+
const searchParams = new SvelteURLSearchParams({
477473
_q: query,
478474
_limit: '5',
479-
cursor: cursor.toString(),
480-
_sort: page.url.searchParams.get('_sort') || '',
481-
_r: page.url.searchParams.get('_r') || ''
475+
cursor: cursor.toString()
482476
});
477+
if (page.url.searchParams.get('_r')) {
478+
searchParams.set('_r', page.url.searchParams.get('_r')!);
479+
}
480+
return searchParams;
483481
}}
484482
transformFn={handleTransform}
485483
extensions={[derivedLxlQualifierPlugin]}
@@ -597,7 +595,6 @@
597595
getCellId,
598596
isFocusedRow,
599597
isFocusedCell
600-
// gotoAfterCollapse
601598
})}
602599
{@const inputRowIndex = 0}
603600
{@const qualifiersRowIndex = showAddQualifiers ? 1 : -1}

‎lxl-web/src/lib/utils/getLabelsFromMapping.svelte.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ function iterateMapping(
6363
keyLabel = el.label;
6464
}
6565
const isLinked = !!el.display?.[JsonLd.ID];
66-
if (isLinked && value === el?._value && el?.displayStr) {
66+
const valueWithoutWrappingParens = value?.trim().match(/^[(].*[)]$/)
67+
? value?.trim().replace(/^[(]/, '').replace(/[)]$/, '')
68+
: value;
69+
if (isLinked && valueWithoutWrappingParens === el?._value && el?.displayStr) {
6770
// only use atomic ranges for linked values
6871
valueLabel = el.displayStr;
6972
removeLink = el.up?.[JsonLd.ID];

‎lxl-web/src/routes/(app)/[[lang=lang]]/AppBar.svelte‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,6 @@
135135
shadowObserver?.disconnect();
136136
}
137137
138-
function handleSubmit(event: SubmitEvent) {
139-
if (document.getElementById('supersearch-dialog')?.hasAttribute('open') && event.target) {
140-
event.preventDefault();
141-
searchContext.submit(event.target as HTMLFormElement);
142-
}
143-
}
144-
145138
beforeNavigate(() => {
146139
closeExpandedMenu();
147140
});
@@ -304,12 +297,7 @@
304297
'hidden target:flex has-[dialog:open]:h-0 lg:flex lg:has-[dialog:open]:h-fit' // enable toggling using target/anchor (so it also works when JavaScript is disabled)
305298
]}
306299
>
307-
<form
308-
id="search-form"
309-
action={findActionUrl}
310-
onsubmit={handleSubmit}
311-
class="mx-auto w-full min-w-0"
312-
>
300+
<form id="search-form" action={findActionUrl} class="mx-auto w-full min-w-0">
313301
{#if isHomeRoute}
314302
<hgroup
315303
class="absolute my-3 px-3 leading-snug @xl:mt-6 lg:@xl:my-3 lg:@xl:px-3 @3xl:leading-normal lg:@3xl:my-3 lg:@3xl:px-4 @5xl:my-4"

‎lxl-web/tests/find.mobile.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ test('mapping displays the correct search query 2', async ({ page }) => {
5151
await page.goto(
5252
'/find?_q=%28contribution:*+OR+%28category:"https://id.kb.se/term/ktg/Literature"+NOT+titel:"pirater"%29%29+existsImage'
5353
);
54+
await page.keyboard.press('Escape'); // why is this needed? Why is the dialog expanded here from start?
5455
await page.getByRole('link', { name: 'Sökfilter' }).click();
5556
const mapping = await page.getByRole('navigation', { name: 'Valda filter' });
5657
const innerText =

‎packages/supersearch/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ To use `supersearch` in a non-Svelte project ...
3838
| `defaultInputCol` | `number` | An integer defining which input item column should be focused by default (use `-1` if no column should be focused). | `-1` |
3939
| `defaultResultItemCol` | `number` | An integer defining which result item column should be focused by default (use `-1` if no column should be focused). | `0` |
4040

41-
| `shallowRouting` | `boolean` | Controls if the component should support the expanded state using the [History API](https://developer.mozilla.org/en-US/docs/Web/API/History_API) (e.g. closing the dialog when pressing back). Links in the expanded search are intercepted so they first go back (collapsing the dialog) before navigating to the `href` attribute of the link. Programmatic usage of `goto` should be replaced with the provided `gotoAfterCollapse` function. | `false` |
41+
| `shallowRouting` | `boolean` | Controls if the component should support the expanded state using the [History API](https://developer.mozilla.org/en-US/docs/Web/API/History_API) (e.g. closing the dialog when pressing back). Links in the expanded search are intercepted so they first go back (collapsing the dialog) before navigating to the `href` attribute of the link. The same applies to form submissions. TODO: Programmatic usage of `goto` should also intercepted similarly. | `false` |
4242
| `toggleWithKeyboardShortcut` | `boolean` | Controls if expanded search should be togglable using `cmd+k`(macOS) and `ctrl+k` (Linux/Windows) | `false` |
4343
| `debouncedWait` | `number` | The wait time, in milliseconds that debounce function should wait between invocated search queries. | `300` |
4444
| `getDebouncedWait` | `DebouncedWaitFunction` | A function `(query: string) => number \| null` for dynamic debounce timing based on query length; return `null` to not fetch at all. | `undefined` |

‎packages/supersearch/e2e/supersearch.spec.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,15 +329,14 @@ test('submits form when pressing submit action', async ({ page }) => {
329329
});
330330

331331
test('submits form on enter key press (if no result item is selected)', async ({ page }) => {
332-
await page.getByRole('combobox').fill('hello world');
333-
await expect(page.getByRole('dialog')).toBeVisible();
334-
await page.keyboard.press('Escape');
332+
await page.getByRole('combobox').click();
333+
await page.getByRole('dialog').getByRole('combobox').fill('hello world');
335334
await page.keyboard.press('Enter');
336335
await expect(page, 'submits closest form').toHaveURL('/test1?q=hello+world');
337336
await page.goBack();
338337
await page.getByTestId('use-form-attribute').check();
339-
await page.getByRole('combobox').fill('hello world');
340-
await page.keyboard.press('Escape');
338+
await page.getByRole('combobox').click();
339+
await page.getByRole('dialog').getByRole('combobox').fill('hello world');
341340
await page.keyboard.press('Enter');
342341
await expect(page, 'submits form specified form attribute').toHaveURL('/test2?q=hello+world');
343342
});

‎packages/supersearch/src/lib/components/SuperSearch.svelte‎

Lines changed: 55 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import { onMount, onDestroy, type Snippet } from 'svelte';
33
import { BROWSER } from 'esm-env';
44
import { page } from '$app/state';
5-
import { goto, pushState } from '$app/navigation';
5+
import { pushState, beforeNavigate } from '$app/navigation';
66
import CodeMirror, {
77
type ChangeCodeMirrorEvent,
88
type SelectCodeMirrorEvent,
@@ -37,7 +37,6 @@
3737
getCellId: (rowIndex: number, cellIndex: number) => string | undefined;
3838
isFocusedCell: (rowIndex: number, cellIndex: number) => boolean;
3939
isFocusedRow: (rowIndex: number) => boolean;
40-
gotoAfterCollapse: (url: string | URL) => void;
4140
};
4241
4342
interface Props {
@@ -72,7 +71,6 @@
7271
onclickSubmit: (event: MouseEvent) => void;
7372
onclickClear: (event: MouseEvent) => void;
7473
onclickClose: (event: MouseEvent) => void;
75-
gotoAfterCollapse?: (url: string | URL) => void;
7674
}
7775
]
7876
>;
@@ -84,7 +82,6 @@
8482
getCellId: (cellIndex: number) => string;
8583
isFocusedCell: (cellIndex: number) => boolean;
8684
rowIndex: number;
87-
gotoAfterCollapse?: (url: string | URL) => void;
8885
}
8986
]
9087
>;
@@ -157,7 +154,9 @@
157154
let expanded = $state(false);
158155
let activeRowIndex: number = $state(0);
159156
let activeColIndex: number = $state(-1);
160-
let gotoAfterCollapseUrl: string | URL | undefined = $state();
157+
let interceptedLinkElement: HTMLLinkElement | undefined = $state(undefined);
158+
let submitLinkElement: HTMLAnchorElement | undefined = $state(undefined);
159+
161160
let prevValue: string = value;
162161
163162
let allowArrowKeyCursorHandling: { vertical: boolean; horizontal: boolean } = $state({
@@ -195,6 +194,12 @@
195194
}
196195
});
197196
197+
beforeNavigate((navigation) => {
198+
if (navigation.to) {
199+
hideExpandedSearch();
200+
}
201+
});
202+
198203
const extensionsWithDefaults = $derived([
199204
keymap.of(standardKeymap), // Needed for atomic ranges to work. Maybe we can use a subset?
200205
preventEnterKeyHandling(),
@@ -303,7 +308,7 @@
303308
}
304309
305310
function handleChangeCodeMirror(event: ChangeCodeMirrorEvent) {
306-
if (!dialog?.open && value !== event.value) {
311+
if (!dialog?.open && value.trim() !== event.value.trim()) {
307312
showExpandedSearch();
308313
}
309314
value = event.value;
@@ -370,33 +375,12 @@
370375
});
371376
}
372377
373-
export async function submit(form: HTMLFormElement) {
374-
hideExpandedSearch();
375-
const formData = new FormData(form);
376-
const formParams = new URLSearchParams(formData as unknown as Record<string, string>);
377-
const actionUrl = new URL(form.action);
378-
gotoAfterCollapse(`${actionUrl.href}?${formParams.toString()}`);
379-
}
380-
381-
async function gotoAfterCollapse(url: string | URL) {
382-
gotoAfterCollapseUrl = url;
383-
history.back();
384-
}
385-
386378
async function handlePopState(event: PopStateEvent) {
379+
interceptedLinkElement?.click();
387380
if (dialog?.open) {
388381
hideExpandedSearch();
389-
}
390-
if (gotoAfterCollapseUrl) {
391-
const url = gotoAfterCollapseUrl;
392-
gotoAfterCollapseUrl = undefined;
393-
await goto(url);
394-
collapsedEditorView?.focus();
395-
} else if (
396-
page.state.expandedSuperSearch &&
397-
event.state['sveltekit:states']?.expandedSuperSearch // a little bit hacky way to ensure the dialog doesn't flicker when navigating forward
398-
) {
399-
showExpandedSearch({ focusRow: 0, preventPushState: true });
382+
} else if (event.state['sveltekit:states']?.expandedSuperSearch) {
383+
showExpandedSearch({ focusRow: 0, preventPushState: true }); // a little bit hacky way to ensure the dialog doesn't flicker when navigating forward
400384
}
401385
}
402386
@@ -463,10 +447,26 @@
463447
: collapsedEditorView?.dom?.closest('form');
464448
465449
if (formElement && formElement instanceof HTMLFormElement) {
466-
if (!shallowRouting) {
467-
hideExpandedSearch();
450+
if (expanded) {
451+
if (shallowRouting) {
452+
const formAction = formElement.getAttribute('action');
453+
const formParams = new URLSearchParams(
454+
new FormData(formElement) as unknown as Record<string, string>
455+
);
456+
if (formAction && formParams) {
457+
submitLinkElement?.setAttribute(
458+
'href',
459+
`${!formAction.startsWith('/') ? '/' : ''}${formAction}?${formParams.toString()}` // A workaround for fixing back/forward navigation when submitting from expanded dialog (link clicks works together with history.back() but not goto() for some reason...)
460+
);
461+
submitLinkElement?.click();
462+
} else {
463+
formElement.requestSubmit();
464+
}
465+
hideExpandedSearch();
466+
}
467+
} else {
468+
formElement.requestSubmit();
468469
}
469-
formElement.requestSubmit();
470470
}
471471
}
472472
@@ -767,14 +767,17 @@
767767
}
768768
769769
function interceptExpandedLinks(event: MouseEvent) {
770-
const target = event.target as HTMLElement;
771-
const linkElement = target.tagName === 'A' ? target : target.closest('a');
770+
if (interceptedLinkElement) {
771+
interceptedLinkElement = undefined; // already intercepted so do nothing
772+
return;
773+
} else {
774+
const target = event.target as HTMLElement;
775+
const linkElement = target.tagName === 'A' ? target : target.closest('a');
772776
773-
if (linkElement) {
774-
const href = linkElement.getAttribute('href');
775-
if (href) {
777+
if (linkElement && linkElement.getAttribute('href')) {
776778
event.preventDefault();
777-
gotoAfterCollapse(href);
779+
interceptedLinkElement = linkElement as HTMLLinkElement;
780+
history.back(); // go back before triggering click again in popstate
778781
}
779782
}
780783
}
@@ -975,6 +978,13 @@
975978
bind:this={comboboxElement}
976979
onclick={interceptExpandedLinks}
977980
>
981+
<!-- The hidden submit link is used as a workaround to achieve correct history navigations when submitting -->
982+
<a
983+
href="/"
984+
class="supersearch-hidden-submit-link"
985+
aria-hidden="true"
986+
bind:this={submitLinkElement}
987+
></a>
978988
{@render inputRow?.({
979989
expanded: true,
980990
inputField: expandedInputSnippet,
@@ -983,8 +993,7 @@
983993
isFocusedRow: () => activeRowIndex === 0,
984994
onclickSubmit: handleClickSubmit,
985995
onclickClear: handleReset,
986-
onclickClose: handleClickClose,
987-
gotoAfterCollapse
996+
onclickClose: handleClickClose
988997
})}
989998
</div>
990999
<!-- svelte-ignore a11y_click_events_have_key_events -->
@@ -997,8 +1006,7 @@
9971006
getCellId: (rowIndex: number, colIndex: number) => `${id}-item-${rowIndex}x${colIndex}`,
9981007
isFocusedCell: (rowIndex: number, colIndex: number) =>
9991008
rowIndex === activeRowIndex && colIndex === activeColIndex,
1000-
isFocusedRow: (rowIndex: number) => rowIndex === activeRowIndex,
1001-
gotoAfterCollapse
1009+
isFocusedRow: (rowIndex: number) => rowIndex === activeRowIndex
10021010
})}
10031011
</div>
10041012
</div>
@@ -1017,4 +1025,8 @@
10171025
.supersearch-suggestions {
10181026
overflow: hidden;
10191027
}
1028+
1029+
.supersearch-hidden-submit-link {
1030+
display: none;
1031+
}
10201032
</style>

0 commit comments

Comments
 (0)