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

feat: update count and refacto code with Signal #203

Merged
merged 8 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ test_front:
@echo "🔎 Running front-end tests..."
${DOCKER_COMPOSE_TEST} run --rm search_nodejs npm run test


test_front_watch:
@echo "🔎 Running front-end tests..."
${DOCKER_COMPOSE_TEST} run --rm search_nodejs npm run test:watch

#-----------#
# Utilities #
Expand Down
17 changes: 0 additions & 17 deletions frontend/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,6 @@ export interface BaseSearchDetail {
searchName: string;
}

/**
* Detail of a search result
*/
export interface SearchResultDetail extends BaseSearchDetail {
results: Object[];
count: number;
pageCount: number;
pageSize: number;
currentPage: number;
facets: Object; // FIXME: we could be more precise
charts: Object; // FIXME: we could be more precise
}

/**
* detail for event asking for a page change
*/
Expand All @@ -29,10 +16,6 @@ export interface ChangePageDetail extends BaseSearchDetail {
* Event to ask for search to be launched
*/
export type LaunchSearchEvent = CustomEvent<BaseSearchDetail>;
/**
* Event notifying a successful search result was received
*/
export type SearchResultEvent = CustomEvent<SearchResultDetail>;
/**
* Event to ask for a new page of a search result
*/
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/layouts/search-layout-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ import {EventRegistrationMixin} from '../event-listener-setup';
import {HIDE_STYLE} from '../styles';
import {ContextConsumer} from '@lit/context';
import {chartSideBarStateContext} from '../context';
import {SearchaliciousResultCtlMixin} from '../mixins/search-results-ctl';

/**
* Component for the layout of the page
* Three columns layout with display flex
*/
@customElement('searchalicious-layout-page')
export class SearchLayoutPage extends EventRegistrationMixin(LitElement) {
export class SearchLayoutPage extends SearchaliciousResultCtlMixin(
EventRegistrationMixin(LitElement)
) {
static override styles = [
HIDE_STYLE,
css`
Expand Down
46 changes: 19 additions & 27 deletions frontend/src/mixins/search-ctl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ import {
EventRegistrationMixin,
} from '../event-listener-setup';
import {QueryOperator, SearchaliciousEvents} from '../utils/enums';
import {
ChangePageEvent,
SearchResultDetail,
SearchResultEvent,
} from '../events';
import {ChangePageEvent} from '../events';
import {Constructor} from './utils';
import {SearchaliciousSort, SortParameters} from '../search-sort';
import {SearchaliciousFacets} from '../search-facets';
Expand All @@ -26,7 +22,12 @@ import {
SearchaliciousHistoryMixin,
} from './history';
import {SearchaliciousChart} from '../search-chart';
import {canResetSearch, isSearchChanged, isSearchLoading} from '../signals';
import {
canResetSearch,
isSearchChanged,
isSearchLoading,
searchResultDetail,
} from '../signals';
import {SignalWatcher} from '@lit-labs/preact-signals';
import {isTheSameSearchName} from '../utils/search';

Expand Down Expand Up @@ -488,6 +489,7 @@ export const SearchaliciousSearchMixin = <T extends Constructor<LitElement>>(
* Launching search
*/
async search(page = 1) {
// use to get the time of the search
const {searchUrl, method, params, history} = this._searchUrl(page);
setCurrentURLHistory(history);

Expand Down Expand Up @@ -518,33 +520,23 @@ export const SearchaliciousSearchMixin = <T extends Constructor<LitElement>>(
isSearchLoading(this.name).value = false;
this.updateSearchSignals();

// dispatch an event with the results
const detail: SearchResultDetail = {
searchName: this.name,
results: this._results!,
count: this._count!,
pageCount: this._pageCount!,
searchResultDetail(this.name).value = {
charts: data.charts,
count: data.count,
currentPage: this._currentPage!,
pageSize: this.pageSize,
displayTime: data.took,
facets: data.facets,
charts: data.charts,
isCountExact: data.is_count_exact,
isSearchLaunch: true,
pageCount: this._pageCount!,
pageSize: this.pageSize,
results: this._results!,
};
this.dispatchEvent(
new CustomEvent(SearchaliciousEvents.NEW_RESULT, {
bubbles: true,
composed: true,
detail: detail,
})
);

this.updateSearchSignals();
}
}

return SearchaliciousSearchMixinClass as Constructor<SearchaliciousSearchInterface> &
T;
};

declare global {
interface GlobalEventHandlersEventMap {
'searchalicious-result': SearchResultEvent;
}
}
71 changes: 17 additions & 54 deletions frontend/src/mixins/search-results-ctl.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,20 @@
import {LitElement} from 'lit';
import {property, state} from 'lit/decorators.js';
import {LitElement, ReactiveElement} from 'lit';
import {property} from 'lit/decorators.js';

import {Constructor} from './utils';
import {DEFAULT_SEARCH_NAME} from '../utils/constants';
import {SearchResultDetail, searchResultDetail} from '../signals';
import {Signal, SignalWatcher} from '@lit-labs/preact-signals';
import {
EventRegistrationInterface,
EventRegistrationMixin,
} from '../event-listener-setup';
import {SearchaliciousEvents} from '../utils/enums';
import {SearchResultEvent} from '../events';
import {Constructor} from './utils';
import {isTheSameSearchName} from '../utils/search';
import {DEFAULT_SEARCH_NAME} from '../utils/constants';

export interface SearchaliciousResultsCtlInterface
extends EventRegistrationInterface {
searchName: string;
searchLaunched: Boolean;

// sub class must override this one to display results
handleResults(event: SearchResultEvent): void;
// this is the registered handler
_handleResults(event: Event): void;
searchResultDetailSignal: Signal<SearchResultDetail>;
searchResultDetail: SearchResultDetail;
}

/**
Expand All @@ -32,8 +27,8 @@ export const SearchaliciousResultCtlMixin = <T extends Constructor<LitElement>>(
/**
* The search mixin, encapsulate the logic of dialog with server
*/
class SearchaliciousResultCtlMixinClass extends EventRegistrationMixin(
superClass
class SearchaliciousResultCtlMixinClass extends SignalWatcher(
EventRegistrationMixin(superClass)
) {
/**
* the search we display result for,
Expand All @@ -42,49 +37,17 @@ export const SearchaliciousResultCtlMixin = <T extends Constructor<LitElement>>(
@property({attribute: 'search-name'})
searchName = DEFAULT_SEARCH_NAME;

/**
* this will be true if we already launched a search
*/
@state()
searchLaunched = false;

handleResults(_: SearchResultEvent) {
throw Error(
'You must provide a handleResults function in you implementation'
);
get searchResultDetailSignal() {
return searchResultDetail(this.searchName);
}

/**
* event handler for NEW_RESULT events
*/
_handleResults(event: Event) {
// check if event is for our search
const resultEvent = event as SearchResultEvent;
if (isTheSameSearchName(this.searchName, event)) {
this.searchLaunched = true;
// call the real method
this.handleResults(resultEvent);
}
}

/**
* Connect search event handlers.
*/
override connectedCallback() {
super.connectedCallback();
this.addEventHandler(SearchaliciousEvents.NEW_RESULT, (event) =>
this._handleResults(event)
);
}
// connect to our specific events
override disconnectedCallback() {
super.disconnectedCallback();
this.removeEventHandler(SearchaliciousEvents.NEW_RESULT, (event) =>
this._handleResults(event)
);
get searchResultDetail() {
return this.searchResultDetailSignal.value;
}
}

return SearchaliciousResultCtlMixinClass as Constructor<SearchaliciousResultsCtlInterface> &
return SearchaliciousResultCtlMixinClass as Constructor<
SearchaliciousResultsCtlInterface & ReactiveElement
> &
T;
};
19 changes: 13 additions & 6 deletions frontend/src/search-chart.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import {LitElement, html} from 'lit';
import {customElement, property} from 'lit/decorators.js';

import {SearchaliciousResultCtlMixin} from './mixins/search-results-ctl';

import {SearchResultEvent} from './events';
import {WHITE_PANEL_STYLE} from './styles';
import {SearchResultDetail} from './signals';
import {SearchaliciousResultCtlMixin} from './mixins/search-results-ctl';

// eslint-disable-next-line
declare const vega: any;
Expand Down Expand Up @@ -32,6 +31,14 @@ export class SearchaliciousChart extends SearchaliciousResultCtlMixin(
this.vegaInstalled = this.testVegaInstalled();
}

override connectedCallback() {
super.connectedCallback();

this.searchResultDetailSignal.subscribe((searchResultDetail) => {
this.updateCharts(searchResultDetail);
});
}

getName() {
return this.name;
}
Expand All @@ -52,14 +59,14 @@ export class SearchaliciousChart extends SearchaliciousResultCtlMixin(
return html` <div class="white-panel">${this.renderChart()}</div>`;
}

override handleResults(event: SearchResultEvent) {
if (event.detail.results.length === 0 || !this.vegaInstalled) {
updateCharts(searchResultDetail: SearchResultDetail) {
if (searchResultDetail.results.length === 0 || !this.vegaInstalled) {
this.vegaRepresentation = undefined;
return;
}

// @ts-ignore
this.vegaRepresentation = event.detail.charts[this.name!];
this.vegaRepresentation = searchResultDetail.charts[this.name!];
}

testVegaInstalled() {
Expand Down
52 changes: 28 additions & 24 deletions frontend/src/search-count.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import {LitElement, html, css} from 'lit';
import {customElement, state} from 'lit/decorators.js';
import {SearchResultEvent} from './events';
import {
SearchaliciousResultCtlMixin,
SearchaliciousResultsCtlInterface,
} from './mixins/search-results-ctl';
import {customElement, property} from 'lit/decorators.js';
import {msg} from '@lit/localize';

import {SearchaliciousResultCtlMixin} from './mixins/search-results-ctl';
@customElement('searchalicious-count')
export class SearchCount
extends SearchaliciousResultCtlMixin(LitElement)
implements SearchaliciousResultsCtlInterface
{
export class SearchCount extends SearchaliciousResultCtlMixin(LitElement) {
static override styles = css`
:host {
display: block;
Expand All @@ -20,9 +12,8 @@ export class SearchCount
}
`;

// the last number of results found
@state()
nbResults = 0;
@property({attribute: 'display-time', type: Boolean})
displayTime = false;

// HTML to display before the search is launched
beforeSearch = html``;
Expand All @@ -32,25 +23,38 @@ export class SearchCount
return html`<div>${msg('No results found')}</div>`;
}

renderResultsFound() {
let result;
if (this.searchResultDetail.isCountExact) {
result = `${this.searchResultDetail.count} ${msg('results found')}`;
} else {
result = msg(
html`More than ${this.searchResultDetail.count} results found`
Kout95 marked this conversation as resolved.
Show resolved Hide resolved
);
}

if (this.displayTime) {
result = html`${result}
<span part="result-display-time">
(${this.searchResultDetail.displayTime}ms)</span
>`;
}

return result;
}

/**
* Render the component
*/
override render() {
if (this.nbResults > 0) {
return this.nbResults + ' results found';
} else if (this.searchLaunched) {
if (this.searchResultDetail.count > 0) {
return html`<div part="result-found">${this.renderResultsFound()}</div>`;
} else if (this.searchResultDetail.isSearchLaunch) {
return html`<slot name="no-results">${this.noResults}</slot>`;
} else {
return html`<slot name="before-search">${this.beforeSearch}</slot>`;
}
}

/**
* Handle the search result event
*/
override handleResults(event: SearchResultEvent) {
this.nbResults = event.detail.count; // it's reactive so it will trigger a render
}
}

declare global {
Expand Down
Loading
Loading