Skip to content

Commit

Permalink
WEBDEV-6918 Fix tile display for default sorted collections (#391)
Browse files Browse the repository at this point in the history
* Fix some weirdness with default sort params

* Ensure item tiles display correct date/views based on effective sort

* Update list & compact list views

* Fix date header in compact list view

* Fix unit test for default sort
  • Loading branch information
latonv authored Aug 30, 2024
1 parent 4b2e04c commit c260465
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 40 deletions.
33 changes: 18 additions & 15 deletions src/collection-browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,6 @@ export class CollectionBrowser
*/
@property({ type: String }) displayMode?: CollectionDisplayMode;

@property({ type: Object }) defaultSortParam: SortParam | null = null;

@property({ type: String }) selectedSort: SortField = SortField.default;

@property({ type: String }) selectedTitleFilter: string | null = null;
Expand All @@ -132,6 +130,13 @@ export class CollectionBrowser

@property({ type: String }) sortDirection: SortDirection | null = null;

@property({ type: String }) defaultSortField: Exclude<
SortField,
SortField.default
> = SortField.relevance;

@property({ type: String }) defaultSortDirection: SortDirection | null = null;

@property({ type: Number }) pageSize = 50;

@property({ type: Number }) currentPage?: number;
Expand Down Expand Up @@ -278,11 +283,6 @@ export class CollectionBrowser

@state() private contentWidth?: number;

@state() private defaultSortField: Exclude<SortField, SortField.default> =
SortField.relevance;

@state() private defaultSortDirection: SortDirection | null = null;

@state() private placeholderType: PlaceholderType = null;

@query('#content-container') private contentContainer!: HTMLDivElement;
Expand Down Expand Up @@ -844,6 +844,17 @@ export class CollectionBrowser
return { field: sortField, direction: this.sortDirection };
}

/**
* An object representing the default sort field & direction, if none are explicitly set.
*/
get defaultSortParam(): SortParam | null {
const direction = this.defaultSortDirection ?? 'asc';
const field = SORT_OPTIONS[this.defaultSortField].searchServiceKey;
if (!field) return null;

return { field, direction };
}

/**
* Handler for when the display mode option is changed (grid/list/compact-list views).
*/
Expand Down Expand Up @@ -1876,10 +1887,6 @@ export class CollectionBrowser
if (sortField && sortField !== SortField.default) {
this.defaultSortField = sortField;
this.defaultSortDirection = dir as SortDirection;
this.defaultSortParam = {
field: this.defaultSortField,
direction: this.defaultSortDirection,
};
}
}

Expand All @@ -1895,10 +1902,6 @@ export class CollectionBrowser
}

this.defaultSortDirection = 'desc';
this.defaultSortParam = {
field: this.defaultSortField,
direction: this.defaultSortDirection,
};
}

/**
Expand Down
5 changes: 2 additions & 3 deletions src/data-source/collection-browser-data-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1025,9 +1025,8 @@ export class CollectionBrowserDataSource
// TODO eventually the PPS should handle these defaults natively
const isDefaultProfileSort =
this.host.withinProfile && this.host.selectedSort === SortField.default;
if (isDefaultProfileSort && this.host.defaultSortParam) {
const sortOption =
SORT_OPTIONS[this.host.defaultSortParam.field as SortField];
if (isDefaultProfileSort && this.host.defaultSortField) {
const sortOption = SORT_OPTIONS[this.host.defaultSortField];
if (sortOption.searchServiceKey) {
sortParams = [
{
Expand Down
2 changes: 1 addition & 1 deletion src/data-source/collection-browser-query-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface CollectionBrowserSearchInterface
extends CollectionBrowserQueryState {
searchService?: SearchServiceInterface;
readonly sortParam: SortParam | null;
readonly defaultSortParam: SortParam | null;
readonly defaultSortField: SortField | null;
readonly facetLoadStrategy: FacetLoadStrategy;
readonly initialPageNumber: number;
readonly currentVisiblePageNumbers: number[];
Expand Down
3 changes: 2 additions & 1 deletion src/tiles/base-tile-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ export abstract class BaseTileComponent extends LitElement {
changed.has('baseNavigationUrl') ||
changed.has('collectionPagePath') ||
changed.has('sortParam') ||
changed.has('defaultSortParam') ||
changed.has('creatorFilter')
) {
this.displayValueProvider = new TileDisplayValueProvider({
model: this.model,
baseNavigationUrl: this.baseNavigationUrl,
collectionPagePath: this.collectionPagePath,
sortParam: this.sortParam ?? undefined,
sortParam: this.sortParam ?? this.defaultSortParam ?? undefined,
creatorFilter: this.creatorFilter,
});
}
Expand Down
16 changes: 13 additions & 3 deletions src/tiles/grid/item-tile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { ifDefined } from 'lit/directives/if-defined.js';
import { msg } from '@lit/localize';

import { map } from 'lit/directives/map.js';
import type { SortParam } from '@internetarchive/search-service';
import { DateFormat, formatDate } from '../../utils/format-date';
import { isFirstMillisecondOfUTCYear } from '../../utils/local-date-from-utc';
import { BaseTileComponent } from '../base-tile-component';
Expand All @@ -28,6 +29,7 @@ export class ItemTile extends BaseTileComponent {
* - baseImageUrl?: string;
* - collectionPagePath?: string;
* - sortParam: SortParam | null = null;
* - defaultSortParam: SortParam | null = null;
* - creatorFilter?: string;
* - mobileBreakpoint?: number;
* - loggedIn = false;
Expand All @@ -37,8 +39,9 @@ export class ItemTile extends BaseTileComponent {

render() {
const itemTitle = this.model?.title;
const effectiveSort = this.sortParam ?? this.defaultSortParam;
const [viewCount, viewLabel] =
this.sortParam?.field === 'week'
effectiveSort?.field === 'week'
? [this.model?.weeklyViewCount, 'weekly views']
: [this.model?.viewCount, 'all-time views'];

Expand Down Expand Up @@ -110,7 +113,7 @@ export class ItemTile extends BaseTileComponent {
private get sortedDateInfoTemplate() {
let sortedValue;
let format: DateFormat = 'long';
switch (this.sortParam?.field) {
switch (this.effectiveSort?.field) {
case 'date': {
const datePublished = this.model?.datePublished;
sortedValue = { field: 'published', value: datePublished };
Expand Down Expand Up @@ -212,10 +215,17 @@ export class ItemTile extends BaseTileComponent {

private get isSortedByDate(): boolean {
return ['date', 'reviewdate', 'addeddate', 'publicdate'].includes(
this.sortParam?.field as string
this.effectiveSort?.field as string
);
}

/**
* Returns the active sort param if one is set, or the default sort param otherwise.
*/
private get effectiveSort(): SortParam | null {
return this.sortParam ?? this.defaultSortParam;
}

private get hasSnippets(): boolean {
return !!this.model?.snippets?.length;
}
Expand Down
2 changes: 1 addition & 1 deletion src/tiles/grid/tile-stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class TileStats extends LitElement {

@property({ type: Number }) viewCount?: number;

@property({ type: String }) viewLabel?: number;
@property({ type: String }) viewLabel?: string;

@property({ type: Number }) favCount?: number;

Expand Down
4 changes: 3 additions & 1 deletion src/tiles/list/tile-list-compact-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ export class TileListCompactHeader extends BaseTileComponent {
<div id="thumb"></div>
<div id="title">${msg('Title')}</div>
<div id="creator">${msg('Creator')}</div>
<div id="date">${this.displayValueProvider.dateLabel}</div>
<div id="date">
${this.displayValueProvider.dateLabel || msg('Published')}
</div>
<div id="icon">${msg('Type')}</div>
<div id="views">${msg('Views')}</div>
</div>
Expand Down
17 changes: 13 additions & 4 deletions src/tiles/list/tile-list-compact.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { css, html, nothing } from 'lit';
import { customElement } from 'lit/decorators.js';
import DOMPurify from 'dompurify';
import type { SortParam } from '@internetarchive/search-service';
import { BaseTileComponent } from '../base-tile-component';

import { formatCount, NumberFormat } from '../../utils/format-count';
Expand All @@ -21,6 +22,7 @@ export class TileListCompact extends BaseTileComponent {
* - baseImageUrl?: string;
* - collectionPagePath?: string;
* - sortParam: SortParam | null = null;
* - defaultSortParam: SortParam | null = null;
* - creatorFilter?: string;
* - mobileBreakpoint?: number;
* - loggedIn = false;
Expand Down Expand Up @@ -91,7 +93,7 @@ export class TileListCompact extends BaseTileComponent {
// In contrast, the search engine metadata uses 'date' to refer to the actual
// publication date of the underlying media ("Date Published" in the UI).
// Refer to the full metadata schema for more info.
switch (this.sortParam?.field) {
switch (this.effectiveSort?.field) {
case 'publicdate':
return this.model?.dateArchived;
case 'reviewdate':
Expand All @@ -104,11 +106,18 @@ export class TileListCompact extends BaseTileComponent {
}

private get views(): number | undefined {
return this.sortParam?.field === 'week'
return this.effectiveSort?.field === 'week'
? this.model?.weeklyViewCount // weekly views
: this.model?.viewCount; // all-time views
}

/**
* Returns the active sort param if one is set, or the default sort param otherwise.
*/
private get effectiveSort(): SortParam | null {
return this.sortParam ?? this.defaultSortParam;
}

private get classSize(): string {
if (
this.mobileBreakpoint &&
Expand All @@ -125,7 +134,7 @@ export class TileListCompact extends BaseTileComponent {
// This is because items with only a year for their publication date are normalized to
// Jan 1 at midnight timestamps in the search engine documents.
if (
(!this.isSortedByDate || this.sortParam?.field === 'date') && // Any sort except dates that aren't published date
(!this.isSortedByDate || this.effectiveSort?.field === 'date') && // Any sort except dates that aren't published date
isFirstMillisecondOfUTCYear(this.model?.datePublished)
) {
return 'year-only';
Expand All @@ -146,7 +155,7 @@ export class TileListCompact extends BaseTileComponent {

private get isSortedByDate(): boolean {
return ['date', 'reviewdate', 'addeddate', 'publicdate'].includes(
this.sortParam?.field as string
this.effectiveSort?.field as string
);
}

Expand Down
21 changes: 15 additions & 6 deletions src/tiles/list/tile-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { customElement, property, state } from 'lit/decorators.js';
import { msg } from '@lit/localize';
import DOMPurify from 'dompurify';

import type { SortParam } from '@internetarchive/search-service';
import { suppressedCollections } from '../../models';
import type { CollectionTitles } from '../../data-source/models';
import { BaseTileComponent } from '../base-tile-component';
Expand All @@ -31,6 +32,7 @@ export class TileList extends BaseTileComponent {
* - baseImageUrl?: string;
* - collectionPagePath?: string;
* - sortParam: SortParam | null = null;
* - defaultSortParam: SortParam | null = null;
* - creatorFilter?: string;
* - mobileBreakpoint?: number;
* - loggedIn = false;
Expand Down Expand Up @@ -224,10 +226,10 @@ export class TileList extends BaseTileComponent {
// Except datePublished which is always shown
private get dateSortByTemplate() {
if (
this.sortParam &&
(this.sortParam.field === 'addeddate' ||
this.sortParam.field === 'reviewdate' ||
this.sortParam.field === 'publicdate')
this.effectiveSort &&
(this.effectiveSort.field === 'addeddate' ||
this.effectiveSort.field === 'reviewdate' ||
this.effectiveSort.field === 'publicdate')
) {
return this.metadataTemplate(
formatDate(this.date, 'long'),
Expand All @@ -239,7 +241,7 @@ export class TileList extends BaseTileComponent {

private get viewsTemplate() {
const viewCount =
this.sortParam?.field === 'week'
this.effectiveSort?.field === 'week'
? this.model?.weeklyViewCount // weekly views
: this.model?.viewCount; // all-time views
if (viewCount == null) return nothing;
Expand Down Expand Up @@ -459,7 +461,7 @@ export class TileList extends BaseTileComponent {
* @see src/models.ts
*/
private get date(): Date | undefined {
switch (this.sortParam?.field) {
switch (this.effectiveSort?.field) {
case 'date':
return this.model?.datePublished;
case 'reviewdate':
Expand All @@ -471,6 +473,13 @@ export class TileList extends BaseTileComponent {
}
}

/**
* Returns the active sort param if one is set, or the default sort param otherwise.
*/
private get effectiveSort(): SortParam | null {
return this.sortParam ?? this.defaultSortParam;
}

private get classSize(): string {
if (
this.mobileBreakpoint &&
Expand Down
12 changes: 8 additions & 4 deletions src/tiles/tile-dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class TileDispatcher
* - baseImageUrl?: string;
* - collectionPagePath?: string;
* - sortParam: SortParam | null = null;
* - defaultSortParam: SortParam | null = null;
* - creatorFilter?: string;
* - mobileBreakpoint?: number;
* - loggedIn = false;
Expand Down Expand Up @@ -111,7 +112,7 @@ export class TileDispatcher
<tile-list-compact-header
class="header"
.currentWidth=${currentWidth}
.sortParam=${sortParam || defaultSortParam}
.sortParam=${sortParam ?? defaultSortParam}
.mobileBreakpoint=${mobileBreakpoint}
>
</tile-list-compact-header>
Expand Down Expand Up @@ -334,7 +335,8 @@ export class TileDispatcher
.currentWidth=${this.currentWidth}
.currentHeight=${this.currentHeight}
.baseImageUrl=${this.baseImageUrl}
.sortParam=${sortParam || defaultSortParam}
.sortParam=${sortParam}
.defaultSortParam=${defaultSortParam}
.creatorFilter=${creatorFilter}
.loggedIn=${this.loggedIn}
.isManageView=${this.isManageView}
Expand All @@ -350,7 +352,8 @@ export class TileDispatcher
.currentWidth=${currentWidth}
.currentHeight=${currentHeight}
.baseNavigationUrl=${baseNavigationUrl}
.sortParam=${sortParam || defaultSortParam}
.sortParam=${sortParam}
.defaultSortParam=${defaultSortParam}
.creatorFilter=${creatorFilter}
.mobileBreakpoint=${mobileBreakpoint}
.baseImageUrl=${this.baseImageUrl}
Expand All @@ -365,7 +368,8 @@ export class TileDispatcher
.currentWidth=${currentWidth}
.currentHeight=${currentHeight}
.baseNavigationUrl=${baseNavigationUrl}
.sortParam=${sortParam || defaultSortParam}
.sortParam=${sortParam}
.defaultSortParam=${defaultSortParam}
.creatorFilter=${creatorFilter}
.mobileBreakpoint=${mobileBreakpoint}
.baseImageUrl=${this.baseImageUrl}
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 @@ -1111,7 +1111,7 @@ describe('Collection Browser', () => {

el.applyDefaultProfileSort();
expect(el.defaultSortParam).to.deep.equal({
field: SortField.weeklyview,
field: 'week',
direction: 'desc',
});
});
Expand Down

0 comments on commit c260465

Please sign in to comment.