Favorite renaming - #30
Conversation
new file: drizzle/0003_favorites_add_name.sql modified: package-lock.json modified: package.json modified: src/__test_utils__/seed.ts modified: src/backend/routes/favorites.test.ts modified: src/backend/routes/favorites.ts modified: src/frontend/cards/card-components.tsx modified: src/frontend/favorites/favorite-button.tsx modified: src/frontend/favorites/favorite-card.tsx modified: src/frontend/favorites/favorite-menu.tsx modified: src/shared/api-models.ts modified: src/shared/schema.ts
new file: src/frontend/favorites/favorite-search.test.ts new file: src/frontend/favorites/favorite-search.ts modified: src/frontend/favorites/favorites-list.tsx modified: src/shared/search.ts
modified: src/frontend/favorites/favorite-menu.tsx
modified: src/backend/routes/favorites.test.ts modified: src/backend/routes/favorites.ts
AlexKempen
left a comment
AlexKempen
left a comment
There was a problem hiding this comment.
It's looking good, you can see my comments above.
I also realized our search logic was pretty scuffed and/or scattered, so I added a typed SearchResultDocument helper and renamed/re-organized our existing search implementation so the organization is a bit clearer, so make sure you run git pull the next time you work on it so you get my changes/don't run into conflicts
| }); | ||
| }; | ||
|
|
||
| const isSearchActive = Boolean(uiState.searchQuery && searchHit); |
There was a problem hiding this comment.
&& is already a boolean, so Boolean() is redundant
|
|
||
| const isSearchActive = Boolean(uiState.searchQuery && searchHit); | ||
| const titleComponent = | ||
| isSearchActive && searchHit ? ( |
There was a problem hiding this comment.
Double checking searchHit (already checked in isSearchActive)
| isSearchActive && searchHit ? ( | ||
| <SearchHitTitle title={favoriteName} searchHit={searchHit} /> | ||
| ) : ( | ||
| <TextInput |
There was a problem hiding this comment.
This TextInput (and related hooks and whatnot) feels like a separate EditTitle component that takes an onEditComplete callback
| const disabled = props.disabled ?? false; | ||
| const isHidden = props.showHiddenTag ?? false; | ||
|
|
||
| let cardTitle: ReactNode; |
There was a problem hiding this comment.
It would probably be best to rename this component CardTitleGroup and have it wrap a different CardTitle component that composes a standard title, SearchHitTitle, and EditTitle components. See https://react.dev/learn/passing-props-to-a-component#passing-jsx-as-children for info on how you can type/use the component, the end pattern you're looking for is:
<CardTitleGroup thumbnailUrls={thumbnailUrls} buildStatusBadge={badge}>
<CardTitle title={title} searchHit={searchHit} isEditing={isEditing} />
<CardTitleGroup />
The EditTitle component can come from your editable title logic below.
| interface FavoriteSearchDocument { | ||
| id: string; | ||
| name: string; | ||
| favoriteId: string; |
There was a problem hiding this comment.
I think it would be good to also include vendors: Vendors[] in here so we can easily display the number of favorites filtered out (the pattern should match the existing pattern). I also added a filter block to the search function you can use to implement
| } | ||
|
|
||
| const FAVORITE_SEARCH_OPTIONS: Options<FavoriteSearchDocument> = { | ||
| fields: ["name", "favoriteId"], |
There was a problem hiding this comment.
Shouldn't need to index fields other than name (and vendors if you add it)
| searchDb.addAll( | ||
| favorites.map((favorite) => ({ | ||
| id: favorite.id, | ||
| name: favorite.name ?? "", |
There was a problem hiding this comment.
We do need the insertable name in the searchDb when favorite name is undefined, which probably means passing in insertables. You could also probably write a helper like getInsertableName(favorite, insertable) which returns "" if the insertable doesn't exist
deleted: coverage/coverage-final.json deleted: coverage/lcov-report/base.css deleted: coverage/lcov-report/block-navigation.js deleted: coverage/lcov-report/favicon.png deleted: coverage/lcov-report/index.html deleted: coverage/lcov-report/prettify.css deleted: coverage/lcov-report/prettify.js deleted: coverage/lcov-report/sort-arrow-sprite.png deleted: coverage/lcov-report/sorter.js deleted: coverage/lcov.info
modified: src/frontend/cards/card-components.tsx modified: src/frontend/cards/insertable-card.tsx modified: src/frontend/favorites/favorite-card.tsx modified: src/frontend/groups/group-card.tsx modified: src/shared/schema.ts
2 participants
Favorites can now be renamed inline. The favorite search now searches by the favorite id and name rather than the insertable. The changes to packages and the addition of the coverage folder were done automatically and I have no idea what they do.