Skip to content

Commit

Permalink
WEBDEV-7150 Ensure cleared query propagates to data source & URL (#412)
Browse files Browse the repository at this point in the history
* Ensure cleared query propagates to data source & URL

* Fix race condition in initial search promise
  • Loading branch information
latonv authored Dec 11, 2024
1 parent beed37a commit 2435e55
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 21 deletions.
9 changes: 4 additions & 5 deletions src/collection-browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1717,15 +1717,14 @@ export class CollectionBrowser
if (
!this.searchService ||
this.dataSource.pageFetchQueryKey === this.previousQueryKey
)
) {
return;
}

// If the new state prevents us from updating the search results, don't reset
if (
!this.dataSource.canPerformSearch &&
!(this.clearResultsOnEmptyQuery && this.baseQuery === '')
)
if (this.baseQuery && !this.dataSource.canPerformSearch) {
return;
}

this.previousQueryKey = this.dataSource.pageFetchQueryKey;

Expand Down
24 changes: 9 additions & 15 deletions src/data-source/collection-browser-data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,18 +149,11 @@ export class CollectionBrowserDataSource
*/
queryErrorMessage?: string;

/**
* Internal property to store the `resolve` function for the most recent
* `initialSearchComplete` promise, allowing us to resolve it at the appropriate time.
*/
private _initialSearchCompleteResolver!: (val: boolean) => void;

/**
* Internal property to store the private value backing the `initialSearchComplete` getter.
*/
private _initialSearchCompletePromise: Promise<boolean> = new Promise(res => {
this._initialSearchCompleteResolver = res;
});
private _initialSearchCompletePromise: Promise<boolean> =
Promise.resolve(true);

/**
* @inheritdoc
Expand Down Expand Up @@ -205,10 +198,9 @@ export class CollectionBrowserDataSource

// We should only reset if either:
// (a) our state permits a valid search, or
// (b) we have a blank query that we want to show empty results for
const shouldShowEmptyQueryResults =
this.host.clearResultsOnEmptyQuery && this.host.baseQuery === '';
if (!(this.canPerformSearch || shouldShowEmptyQueryResults)) return;
// (b) we have a blank query that we're showing a placeholder/message for
const queryIsEmpty = !this.host.baseQuery;
if (!(this.canPerformSearch || queryIsEmpty)) return;

if (this.activeOnHost) this.host.emitQueryStateChanged();
this.handleQueryChange();
Expand Down Expand Up @@ -238,6 +230,7 @@ export class CollectionBrowserDataSource
this.yearHistogramAggregation = undefined;
this.pageElements = undefined;
this.parentCollections = [];
this.previousQueryKey = '';
this.queryErrorMessage = undefined;

this.offset = 0;
Expand Down Expand Up @@ -382,8 +375,9 @@ export class CollectionBrowserDataSource
this.reset();

// Reset the `initialSearchComplete` promise with a new value for the imminent search
let initialSearchCompleteResolver: (value: boolean) => void;
this._initialSearchCompletePromise = new Promise(res => {
this._initialSearchCompleteResolver = res;
initialSearchCompleteResolver = res;
});

// Fire the initial page & facet requests
Expand All @@ -394,7 +388,7 @@ export class CollectionBrowserDataSource
]);

// Resolve the `initialSearchComplete` promise for this search
this._initialSearchCompleteResolver(true);
initialSearchCompleteResolver!(true);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/collection-browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1010,11 +1010,11 @@ describe('Collection Browser', () => {
const el = await fixture<CollectionBrowser>(
html`<collection-browser
.searchService=${searchService}
@searchResultsLoadingChanged=${spy}
></collection-browser>`
);

el.baseQuery = 'collection:foo';
el.addEventListener('searchResultsLoadingChanged', spy);
await el.updateComplete;
await el.initialSearchComplete;

Expand Down

0 comments on commit 2435e55

Please sign in to comment.