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

Enhancement - collection do not render empty filter #972

Merged
merged 24 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ff004f4
make 'others' a const
adriangohjw Jan 2, 2025
4e5d54f
extract into consts
adriangohjw Jan 2, 2025
94a6767
extract into getYearFilters
adriangohjw Jan 2, 2025
16cdd63
extract getCategoryFilters
adriangohjw Jan 2, 2025
b355073
add correct types
adriangohjw Jan 2, 2025
cd0a7bf
extract getTagFilters
adriangohjw Jan 2, 2025
d3df2ed
rename to singular
adriangohjw Jan 2, 2025
92c5a6d
move logic into each get*Filter function to improve readability
adriangohjw Jan 2, 2025
368ad74
fix import
adriangohjw Jan 2, 2025
1571251
remove type renaming during import
adriangohjw Jan 2, 2025
929d778
refactor imports location
adriangohjw Jan 2, 2025
c7d7c85
refactor: standardize code structure
adriangohjw Jan 2, 2025
7cc0190
only return filters with at least 2 items
adriangohjw Jan 2, 2025
2a4720b
add storybook
adriangohjw Jan 2, 2025
cf20aa4
do not render "category" if there's no category filter
adriangohjw Jan 2, 2025
7748466
Update packages/components/src/templates/next/layouts/Collection/util…
adriangohjw Jan 3, 2025
5d17dd2
Merge remote-tracking branch 'origin/HEAD' into enhancement-collectio…
adriangohjw Jan 3, 2025
c5ab110
split methods in utils into utils folder
adriangohjw Jan 3, 2025
6611f6e
sort tag filters
adriangohjw Jan 3, 2025
97e2854
Merge remote-tracking branch 'origin/main' into enhancement-collectio…
adriangohjw Jan 3, 2025
c18d9e9
undo: shouldShowCategory
adriangohjw Jan 3, 2025
d8b685d
should filter out any filters with no items
adriangohjw Jan 3, 2025
dc06a13
add tests
adriangohjw Jan 3, 2025
169fe49
update storybook
adriangohjw Jan 3, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,39 @@ export const AllResultsNoDate: Story = {
},
}

export const AllResultsSameCategory: Story = {
name: "Should show category filter even if all items have same category",
args: generateArgs({
collectionItems: COLLECTION_ITEMS.map((item) => ({
...item,
category: "The only categ0ry",
})),
}),
play: async ({ canvasElement }) => {
const screen = within(canvasElement)
const categoryFilter = screen.queryByText(/Category/i)
await expect(categoryFilter).toBeInTheDocument()

const categoryItems = await screen.findAllByText(/The only categ0ry \(30\)/)
await expect(categoryItems.length).toBe(1)
},
}

export const AllResultsSameYear: Story = {
name: "Should show year filter if all items have same year",
args: generateArgs({
collectionItems: COLLECTION_ITEMS.map((item) => ({
...item,
date: "2026-05-07",
})),
}),
play: async ({ canvasElement }) => {
const screen = within(canvasElement)
const yearFilter = screen.queryByText(/Year/i)
await expect(yearFilter).toBeInTheDocument()
},
}

export const FileCard: Story = {
args: generateArgs({
collectionItems: [COLLECTION_ITEMS[1]] as IsomerSitemap[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { Skeleton } from "../Skeleton"
import CollectionClient from "./CollectionClient"
import { getAvailableFilters, shouldShowDate } from "./utils"

const CATEGORY_OTHERS = "Others"

const getCollectionItems = (
site: IsomerSiteProps,
permalink: string,
Expand Down Expand Up @@ -62,7 +64,7 @@ const getCollectionItems = (
type: "collectionCard" as const,
rawDate: date,
lastUpdated: date?.toISOString(),
category: item.category || "Others",
category: item.category || CATEGORY_OTHERS,
title: item.title,
description: item.summary,
image: item.image,
Expand Down
256 changes: 0 additions & 256 deletions packages/components/src/templates/next/layouts/Collection/utils.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { describe, expect, it } from "vitest"

import type { ProcessedCollectionCardProps } from "~/interfaces"
import { getCategoryFilter } from "../getCategoryFilter"

describe("getCategoryFilter", () => {
it("should return empty filter items when no items provided", () => {
// Arrange
const items: ProcessedCollectionCardProps[] = []

// Act
const result = getCategoryFilter(items)

// Assert
expect(result).toEqual({
id: "category",
label: "Category",
items: [],
})
})

it("should count and format categories correctly", () => {
// Arrange
const items: ProcessedCollectionCardProps[] = [
{
category: "guides",
} as ProcessedCollectionCardProps,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: this type-cast is a little weird. I understand that we just want to test specific parts of the collection card props but this might hide problems if the implementation were to be changed, or if the props has changed upstream.

We can probably create a few complete test objects to be used across the different tests, or have some builder function that will automatically populate the remaining fields so that we can avoid this type casting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might hide problems if the implementation were to be changed, or if the props has changed upstream.

hmm not quite understanding this, as this test should fail if the implementation changes (e.g. using more props), which should be the intended outcome of a test?

{
category: "guides",
} as ProcessedCollectionCardProps,
{
category: "articles",
} as ProcessedCollectionCardProps,
{
category: "tutorials",
} as ProcessedCollectionCardProps,
{
category: "tutorials",
} as ProcessedCollectionCardProps,
]

// Act
const result = getCategoryFilter(items)

// Assert
expect(result).toEqual({
id: "category",
label: "Category",
items: [
{ id: "articles", label: "Articles", count: 1 },
{ id: "guides", label: "Guides", count: 2 },
{ id: "tutorials", label: "Tutorials", count: 2 },
],
})
})
})
Loading
Loading