Search page #73

Emmanuel-Develops · 2024-12-20T11:49:19Z

vercel · 2024-12-20T11:49:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
registry	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 15, 2025 3:11pm

0tuedon · 2025-01-08T09:27:15Z

Asides the niit from the call? ready for review? @Emmanuel-Develops

Emmanuel-Develops · 2025-01-08T10:22:44Z

Asides the niit from the call? ready for review? @Emmanuel-Develops

core functionality, yes.
There's still some ui/ux nits to work out

IgboPharaoh · 2025-01-09T04:25:49Z

Hello @Emmanuel-Develops can you please rebase your branch to the latest on staging to resolve merge conflicts

Emmanuel-Develops · 2025-01-10T07:52:13Z

Hello @Emmanuel-Develops can you please rebase your branch to the latest on staging to resolve merge conflicts

rebased 👍

…ilter menu

src/components/search/SearchResultCard.tsx

src/utils/search.ts

kouloumos

The implementation looks good overall, and I was able to test it locally. Search functionality works as expected.

However, I noticed a few issues and potential areas for improvement:

Testing Prerequisites

To test locally, you need to define the API_KEY and CLOUD_ID environment variables. This requirement should be mentioned in the README, similar to how we handle it for bitcoin-search.

Code Reuse

For files copied fully or partially from bitcoin-search, it would be helpful to include a note at the top of the file with a reference link. This adds context for future developers.

Additionally, if there's significant duplicated logic, we should consider centralizing it. With TLDR coming (which will also have a search page), maintaining similar functionality across three repositories is not sustainable.

Observations

A couple of issues I observed:

Topics Filtering

search query /search?search=dlcs sir

I see duplicate values for the very first tags, first tag filters correctly, the other displays a 404 page in the search results.

There seems to be an issue with keeping state in the searchbox when using search

Those issues don't exist in the Speakers and Sources filters.

Red colors

Is the red text for "No matching options" and the red "X" for the search part in the designs? They look out of place.

Result Card

The date placement on the result card does not align with the design specifications and looks awkward.

We should make a note for the source on the card. I know that is not an easy fix, but the way that is being displayed (slug instead of name) is not correct.

Pagination isn't centered

kouloumos · 2025-01-13T13:15:52Z

src/config/index.ts

+  "title",
+  "body",
+  "authors",
+  "tags",
+  "summary",
+  "transcript_source",
+];


For Bitcoin Search we are only search authors, title and body. My thoughts:

I understand the addition of transcript_source.

I'm not sure about the addition of tags and summary. If they are actually useful, then we might need to consider adding them to Bitcoin Search as well.

Good point on summary it really isn't useful. However tags do offer some benefits, especially for single search keywords that are topics that may not have the topic name in the body.

src/config/index.ts

src/components/search/SearchResultCard.tsx

src/config/index.ts

Emmanuel-Develops · 2025-01-14T10:21:43Z

For files copied fully or partially from bitcoin-search, it would be helpful to include a note at the top of the file with a reference link. This adds context for future developers.

Code that can be said to be partially duplicated are useURLManager and useSearch, and these hooks are self-contained functions (containing some different variables and functions in this implementation). Linking them to bitcoin-search doesn't provide any extra context.

Only /search/api contains more nuanced search, and I do not include it as it will be deleted during refactoring to use your global search API.
Notice in https://github.com/bitcointranscripts/registry/blob/search-page/src/app/search/searchCall.ts#L24 it can make a call to a provided URL rather than /search/api all custom data fetching will be deleted in favor of the global search API endpoint

also the envs won't be required anymore

Emmanuel-Develops · 2025-01-14T10:44:47Z

I see duplicate values for the very first tags, first tag filters correctly, the other displays a 404 page in the search results.

I am not getting the same results as you are (can't reproduce)

Is the red text for "No matching options" and the red "X" for the search part in the designs? They look out of place.

Mmm, there are no designs for this. Let's discuss that in the dev call today.
I'll switch it to grey in the meantime, but this isn't a blocker 👍

The date placement on the result card does not align with the design specifications and looks awkward.

fixed!

Pagination isn't centered

fixed!

cc: @kouloumos

… use summary

…itle for topics facet

…loated data from landing page

kouloumos

Amazing! All my previous comments have been covered. Additionally, all of the places where "slug" was used, we are now using the full names.

This is the last one to be mapped, in the "Applied Filters":

I think that it should be a quick fix by passing the selected filter from the same mapping function.

ACK as soon as the above is fixed.

…e in applied filters

Emmanuel-Develops · 2025-01-15T13:54:03Z

Amazing! All my previous comments have been covered. Additionally, all of the places where "slug" was used, we are now using the full names.

This is the last one to be mapped, in the "Applied Filters":

I think that it should be a quick fix by passing the selected filter from the same mapping function.

ACK as soon as the above is fixed.

thanks, pushed the fix

kouloumos · 2025-01-15T14:06:53Z

Looks good to me! 🚀

* feat: base search * feat: link pills, speakers and topic cards to search * feat: mobile responsiveness * fix: cleanups and loader * fix: sort fields and query * refactor: switch useSearch to context from hook * fix: prevent deopt to CSR with suspense * feat: clickable pills either as link or function * feat: loaders * fix: use tagsDetailed from rebase * fix: search behaviour logic and search results ui * refactor: break search box into mobile component, standalone mobile filter menu * fix(hydration-error): Pills as buttons on featured transcripts * fix(review-fixes): source naming, result card UI, pagination, readme, use summary * fix(review-fixes): searchbox clear on page navigation and get topic title for topics facet * feat(optimizations): too many unnecessary prefetch requests, remove bloated data from landing page * fix(search): prevent overflow on search results, add mapped topic name in applied filters * fix(mobile): quality of life improvements

Emmanuel-Develops force-pushed the search-page branch from 230046c to 56ec146 Compare December 20, 2024 11:51

vercel bot had a problem deploying to Preview December 20, 2024 11:54 Failure

Emmanuel-Develops force-pushed the search-page branch from 56ec146 to 9cdd69e Compare January 7, 2025 13:46

vercel bot deployed to Preview January 7, 2025 13:50 View deployment

Emmanuel-Develops marked this pull request as ready for review January 7, 2025 14:11

Emmanuel-Develops requested review from IgboPharaoh, 0tuedon and kouloumos January 7, 2025 14:13

Emmanuel-Develops force-pushed the staging branch from f5e214e to 358e47a Compare January 8, 2025 14:45

Emmanuel-Develops added 10 commits January 10, 2025 08:50

feat: base search

b03ea51

feat: link pills, speakers and topic cards to search

c24dadc

feat: mobile responsiveness

6008574

fix: cleanups and loader

bfc2341

fix: sort fields and query

c2fae98

refactor: switch useSearch to context from hook

b030f4d

fix: prevent deopt to CSR with suspense

ee6782e

feat: clickable pills either as link or function

0b48309

feat: loaders

908695c

fix: use tagsDetailed from rebase

ce406f5

Emmanuel-Develops force-pushed the search-page branch from 9cdd69e to ce406f5 Compare January 10, 2025 07:51

vercel bot deployed to Preview January 10, 2025 07:55 View deployment

Emmanuel-Develops added 2 commits January 10, 2025 20:31

fix: search behaviour logic and search results ui

a250600

refactor: break search box into mobile component, standalone mobile f…

10902c1

…ilter menu

vercel bot deployed to Preview January 10, 2025 19:36 View deployment

0tuedon requested changes Jan 13, 2025

View reviewed changes

src/components/search/SearchResultCard.tsx Show resolved Hide resolved

src/utils/search.ts Outdated Show resolved Hide resolved

fix(hydration-error): Pills as buttons on featured transcripts

12023bc

vercel bot deployed to Preview January 13, 2025 14:35 View deployment

kouloumos reviewed Jan 13, 2025

View reviewed changes

vercel bot deployed to Preview January 14, 2025 10:47 View deployment

fix(review-fixes): source naming, result card UI, pagination, readme,…

729346b

… use summary

Emmanuel-Develops force-pushed the search-page branch from 1ced50b to 729346b Compare January 14, 2025 11:28

vercel bot deployed to Preview January 14, 2025 11:31 View deployment

Emmanuel-Develops added 2 commits January 15, 2025 01:53

fix(review-fixes): searchbox clear on page navigation and get topic t…

233728b

…itle for topics facet

feat(optimizations): too many unnecessary prefetch requests, remove b…

4b1fff9

…loated data from landing page

vercel bot deployed to Preview January 15, 2025 01:01 View deployment

kouloumos approved these changes Jan 15, 2025

View reviewed changes

fix(search): prevent overflow on search results, add mapped topic nam…

adc695e

…e in applied filters

vercel bot deployed to Preview January 15, 2025 13:56 View deployment

fix(mobile): quality of life improvements

6cc9c6d

vercel bot deployed to Preview January 15, 2025 15:11 View deployment

Emmanuel-Develops merged commit 5fc22f8 into staging Jan 15, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search page #73

Search page #73

Emmanuel-Develops commented Dec 20, 2024 •

edited

Loading

vercel bot commented Dec 20, 2024 •

edited

Loading

0tuedon commented Jan 8, 2025

Emmanuel-Develops commented Jan 8, 2025

IgboPharaoh commented Jan 9, 2025

Emmanuel-Develops commented Jan 10, 2025

kouloumos left a comment

kouloumos Jan 13, 2025

Emmanuel-Develops Jan 14, 2025

Emmanuel-Develops commented Jan 14, 2025

Emmanuel-Develops commented Jan 14, 2025

kouloumos left a comment

Emmanuel-Develops commented Jan 15, 2025

kouloumos commented Jan 15, 2025

Search page #73

Search page #73

Conversation

Emmanuel-Develops commented Dec 20, 2024 • edited Loading

vercel bot commented Dec 20, 2024 • edited Loading

0tuedon commented Jan 8, 2025

Emmanuel-Develops commented Jan 8, 2025

IgboPharaoh commented Jan 9, 2025

Emmanuel-Develops commented Jan 10, 2025

kouloumos left a comment

Choose a reason for hiding this comment

Testing Prerequisites

Code Reuse

Observations

Topics Filtering

Red colors

Result Card

Pagination isn't centered

kouloumos Jan 13, 2025

Choose a reason for hiding this comment

Emmanuel-Develops Jan 14, 2025

Choose a reason for hiding this comment

Emmanuel-Develops commented Jan 14, 2025

Emmanuel-Develops commented Jan 14, 2025

kouloumos left a comment

Choose a reason for hiding this comment

Emmanuel-Develops commented Jan 15, 2025

kouloumos commented Jan 15, 2025

Emmanuel-Develops commented Dec 20, 2024 •

edited

Loading

vercel bot commented Dec 20, 2024 •

edited

Loading