MRKT-14: search sales by typing #632

scandycuz · 2024-05-19T19:22:14Z

No description provided.

github-actions · 2024-05-19T19:24:46Z

Visit the preview URL for this PR (updated for commit ac11ed2):

https://newm-artist-portal--pr632-mrkt-14-search-sales-um90ys2g.web.app

_{(expires Fri, 21 Jun 2024 03:59:34 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: ec3483d4c62309afc398865bfd6a9fc9e03e1d46}

github-actions · 2024-05-19T19:24:55Z

Visit the preview URL for this PR (updated for commit ac11ed2):

https://fan-square--pr632-mrkt-14-search-sales-48t73add.web.app

_{(expires Fri, 21 Jun 2024 04:02:36 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: b38f4676e459f355dcb11c95730012d43389bfd2}

github-actions · 2024-05-19T19:31:19Z

Visit the wallet preview URL for this PR (updated for commit dfef51c):
🚀 https://6e4wvmqk4n3wzptxm7fh27o63u0qrpkc.lambda-url.us-west-2.on.aws/

github-actions · 2024-05-19T19:31:28Z

Visit the marketplace preview URL for this PR (updated for commit dfef51c):
🚀 https://jbk2bo5n6vimun5whtamqtx56y0vsqfh.lambda-url.us-west-2.on.aws/

scandycuz · 2024-05-19T21:05:11Z

packages/components/src/lib/SongCard.tsx

    event.preventDefault();
    event.stopPropagation();
-    onSubtitleClick?.();
+    onSubtitleClick();


Updated these so that clicking the element doesn't prevent the element beneath it from being clicked when handler isn't present.

github-actions · 2024-05-19T21:11:11Z

Visit the marketplace preview URL for this PR (updated for commit 340441d):
🚀 https://jbk2bo5n6vimun5whtamqtx56y0vsqfh.lambda-url.us-west-2.on.aws/

github-actions · 2024-05-19T21:11:35Z

Visit the wallet preview URL for this PR (updated for commit 340441d):
🚀 https://6e4wvmqk4n3wzptxm7fh27o63u0qrpkc.lambda-url.us-west-2.on.aws/

github-actions · 2024-05-19T21:55:22Z

Visit the wallet preview URL for this PR (updated for commit aec7f3a):
🚀 https://6e4wvmqk4n3wzptxm7fh27o63u0qrpkc.lambda-url.us-west-2.on.aws/

github-actions · 2024-05-19T21:55:23Z

Visit the marketplace preview URL for this PR (updated for commit aec7f3a):
🚀 https://jbk2bo5n6vimun5whtamqtx56y0vsqfh.lambda-url.us-west-2.on.aws/

dmkirshon · 2024-05-20T13:51:44Z

apps/marketplace/src/components/Sales.tsx

+            { typeof noResultsContent === "string" ? (
+              <Typography sx={ { marginTop: 8, textAlign: "center" } }>
+                { noResultsContent }
+              </Typography>
+            ) : (
+              noResultsContent
+            ) }


Is this just prep for some other content? Is design/product working on something to place here instead of the text?

There isn't a mock-up for when the search page doesn't return any results, but I implemented it so that it has custom text when no results are returned. It's also left aligned, so it's passed in as a React node with styling to make it left aligned instead of just a string, which would be center aligned by default.

Another way to do it could be to have a noResultsContent string prop, and a noResultsContentAlignment prop to set the text alignment, but it seemed easier to just allow passing a single React node prop.

dmkirshon

LGTM

escobarjonatan · 2024-05-21T02:44:19Z

apps/marketplace/src/app/search/page.tsx

-          onSubtitleClick={ () => {} }
-        />
-      </Stack>
+      <SearchResults query={ searchTerm } />
    </Container>
  );
 };


I think we should consider this if !searchTerm is present.

"use client"; import { Container } from "@mui/material"; import { useRouter, useSearchParams } from "next/navigation"; import { SearchResults } from "../../components"; import { useAppDispatch } from "../../common"; import { setToastMessage } from "../../modules/ui"; const Search = () => { const searchParams = useSearchParams(); const dispatch = useAppDispatch(); const router = useRouter(); const searchTerm = searchParams.get("searchTerm") || ""; if (!searchTerm) { dispatch( setToastMessage({ message: "No search term provided. Please enter a search term and try again.", severity: "error", }) ); router.push("/"); } return ( <Container sx={ { flexGrow: 1, mt: 5 } }> <SearchResults query={ searchTerm } /> </Container> ); }; export default Search; useSearchParams(); const dispatch = useAppDispatch(); const router = useRouter(); const searchTerm = searchParams.get("searchTerm") || ""; if (!searchTerm) { dispatch( setToastMessage({ message: "There is a problem with your wallet. Please try disconnecting and reconnecting it.", severity: "error", }) ); router.push("/"); } return ( <Container sx={ { flexGrow: 1, mt: 5 } }> <SearchResults query={ searchTerm } /> </Container> ); }; export default Search;

The current implementation has unreachable code: return null; (also gets called out by my IDE). It also leaves the app in an error state that the user might not know what to do?

With the changes suggested:

Display a toast message letting them know the error

Redirect the user to the home page (that also has the search available)

It's strange the linter is throwing an error about retuning a null value, it shouldn't do that if the null value will only be returned in some cases. In retrospect, I'm thinking we can probably just return an empty result if the search term is empty or missing. The user shouldn't actually ever be able to see that page with an empty or missing search string, and if they do, by manually changing the url, then I think just displaying a message that no results were returned should work.

escobarjonatan · 2024-05-21T03:13:00Z

apps/marketplace/src/components/Sales.tsx

@@ -75,7 +85,6 @@ const Sales: FunctionComponent<SalesProps> = ({
                  title={ song.title }
                  onCardClick={ () => handleCardClick(id) }
                  onPlayPauseClick={ () => playPauseAudio(song.clipUrl) }
-                  onSubtitleClick={ () => handleSubtitleClick(id) }


I see why you are removing it, however, instead of removing it we should change the subtitle from the current genresString to subtitle={ song.artistName } And on subtitle click it would take the user to the artist page. So these changes:

Replace:
subtitle={ song.artistName }
to
subtitle={ genresString }

Replace:
onSubtitleClick={ () => handleSubtitleClick(id) }
to
onSubtitleClick={ () => handleSubtitleClick(song.artistId) }

Ah, yeah. Seems the mock-ups were updated.

escobarjonatan · 2024-05-21T03:15:54Z

apps/marketplace/src/app/search/page.tsx

@@ -1,50 +1,21 @@
 "use client";


There’s a weird bug in I noticed.

Steps:

In the search bar of the home page, type w and and submit the search

Now add 1 space before the letter w, so this w and submit, the url should look something like this: http://localhost:3000/search/?searchTerm=%20w

Add 1 more space before the space and w so this: w and submit, the url should now look like this: http://localhost:3000/search/?searchTerm=%20%20w

Notice how the UI is showing 1 item(screenshot), but the network call returned an empty array

This looks to be because that song is a duplicate in the test database, which is causing issues with the React rendering because they have the same song ID, which is being used as the key prop. I updated the key to be the sale ID, which is unique and resolved the issue.

ahh, makes sense, i saw that key issue somewhere else in the console

github-actions · 2024-05-22T04:06:13Z

Visit the marketplace preview URL for this PR (updated for commit 2377720):
🚀 https://jbk2bo5n6vimun5whtamqtx56y0vsqfh.lambda-url.us-west-2.on.aws/

github-actions · 2024-05-22T04:06:16Z

Visit the wallet preview URL for this PR (updated for commit 2377720):
🚀 https://6e4wvmqk4n3wzptxm7fh27o63u0qrpkc.lambda-url.us-west-2.on.aws/

escobarjonatan · 2024-05-22T04:11:56Z

apps/marketplace/src/app/search/page.tsx

@@ -1,50 +1,21 @@
 "use client";


ahh, makes sense, i saw that key issue somewhere else in the console

scandycuz added 3 commits May 19, 2024 12:01

implements sale search with actual data

b382899

prevents clicking song card subtitle from preventing card click

2135df4

throws error if search missing query param

c1669fe

moves no search term present to search page component

2bf104c

scandycuz requested review from escobarjonatan and dmkirshon May 19, 2024 21:02

scandycuz commented May 19, 2024

View reviewed changes

updates name of SearchResults component

79123ae

dmkirshon reviewed May 20, 2024

View reviewed changes

dmkirshon approved these changes May 20, 2024

View reviewed changes

escobarjonatan reviewed May 21, 2024

View reviewed changes

scandycuz added 2 commits May 21, 2024 20:51

misc updates

aadcf9f

merges in master

ac11ed2

escobarjonatan approved these changes May 22, 2024

View reviewed changes

scandycuz merged commit 2db6ff8 into master May 22, 2024
7 checks passed

scandycuz deleted the MRKT-14/search-sales-by-typing branch May 22, 2024 04:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MRKT-14: search sales by typing #632

MRKT-14: search sales by typing #632

scandycuz commented May 19, 2024

github-actions bot commented May 19, 2024 •

edited

Loading

github-actions bot commented May 19, 2024 •

edited

Loading

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

scandycuz May 19, 2024

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

dmkirshon May 20, 2024

scandycuz May 22, 2024 •

edited

Loading

dmkirshon left a comment

escobarjonatan May 21, 2024

scandycuz May 22, 2024 •

edited

Loading

escobarjonatan May 21, 2024

scandycuz May 22, 2024

escobarjonatan May 21, 2024

scandycuz May 22, 2024 •

edited

Loading

escobarjonatan May 22, 2024

github-actions bot commented May 22, 2024

github-actions bot commented May 22, 2024

escobarjonatan May 22, 2024

MRKT-14: search sales by typing #632

MRKT-14: search sales by typing #632

Conversation

scandycuz commented May 19, 2024

github-actions bot commented May 19, 2024 • edited Loading

github-actions bot commented May 19, 2024 • edited Loading

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

Choose a reason for hiding this comment

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

github-actions bot commented May 19, 2024

Choose a reason for hiding this comment

scandycuz May 22, 2024 • edited Loading

Choose a reason for hiding this comment

dmkirshon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scandycuz May 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scandycuz May 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 22, 2024

github-actions bot commented May 22, 2024

Choose a reason for hiding this comment

github-actions bot commented May 19, 2024 •

edited

Loading

github-actions bot commented May 19, 2024 •

edited

Loading

scandycuz May 22, 2024 •

edited

Loading

scandycuz May 22, 2024 •

edited

Loading

scandycuz May 22, 2024 •

edited

Loading