[Search page]: Filter last records carousel by producer #128
Conversation
cmoinier
force-pushed
the
optional-producer-filter
branch
from
07b526a to
bab1cf6
Compare
cmoinier
force-pushed
the
optional-producer-filter
branch
from
bab1cf6 to
df2c44c
Compare
cmoinier
force-pushed
the
optional-producer-filter
branch
from
7e181b5 to
83f40a0
Compare
cmoinier
force-pushed
the
optional-producer-filter
branch
from
c3b48e7 to
f8f30d0
Compare
There was a problem hiding this comment.
Thanks, @cmoinier, the implementation works really well!
I'm just wondering if it's the right approach to filter the results in the search-header.component and than pass them down to the list. In this case, there is no need to keep the headerSearch state, I think.
However, I have the impression it could be simpler and cleaner to just pass down the filter values to the headerSearch state and than let the result-list handle the synchronization of the producer filter values. Not sure if this is feasable. WDYT?
| @@ -16,6 +16,7 @@ <h1 class="flex mel-page-title md:mb-12" translate=""> | |||
| </div> | |||
| <div class="h-64" gnUiSearchStateContainer="headerSearch"> | |||
| <mel-datahub-results-list-carousel | |||
| [records]="records$ | async" | |||
There was a problem hiding this comment.
I'm wondering if you could just pass the filter values from the mainSearch search state down to the results-list.comonent (which has its own headerSearch search state) here and then make the results-list.component listen to the producer changes.
| JSON.stringify(currentProducerFilter) !== | ||
| JSON.stringify(previousProducerFilter) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Not sure if all the logic in this component is necessary and if it's the component's responsibility to handle this. Maybe we can leverage the separate headerSearch state and achieve to only synchronize the producer filter of the mainSearch state with it by passing down its value as mentioned above. WDYT?
| ) {} | ||
|
|
||
| producerHasChanged$ = this.searchFacade.searchFilters$.pipe( | ||
| distinctUntilChanged(), | ||
| debounceTime(100), |
There was a problem hiding this comment.
Is the debounce (still) needed here?
| @@ -19,6 +19,7 @@ <h1 class="flex justify-center mel-page-title" translate=""> | |||
| </div> | |||
| </div> | |||
| <mel-datahub-results-list-carousel | |||
| [records]="records" | |||
| @@ -27,6 +27,7 @@ import { SortByField } from 'geonetwork-ui/libs/common/domain/src/lib/model/sear | |||
| }) | |||
| export class HomeHeaderComponent { | |||
| HREF_ROUTE_SEARCH = `${DATAHUB_ROOT}/${DATAHUB_ROUTE_SEARCH}` | |||
| @Input() records: CatalogRecord[] = [] | |||
There was a problem hiding this comment.
Same here, shouldn't be needed anymore.
| <mel-datahub-home-header | ||
| [records]="searchFacade.results$ | async" | ||
| ></mel-datahub-home-header> |
| export class HomePageComponent { | ||
| constructor(protected searchFacade: SearchFacade) {} | ||
| } |
| export class ResultsListCarouselComponent extends ResultsListComponent { | ||
| } |
This PR adds a filtering on the last records carousel by producer.
Key features
search.cyhas had a.onlystatement left, so most tests were failing when I removed it. They are now fixed but that's why the file has many changes. Removed the quality score test because this filter is not being used.