feat: [SC-26202] ✨ Build Name Details Page draft and most important … #570
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Skipped Deployments
|
FrancoAguzzi
requested review from
djstrong,
Carbon225,
notrab,
lightwalker-eth,
BanaSeba and
eduramme
as code owners
🦋 Changeset detectedLatest commit: b9248d8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
* feat: [SC-26223] ✨ Migrate NFTAvatar to namekit-react * feat: [sc-26178] Create readme for NameAI SDK (#578) * create readme for nameai-sdk; add license * extend response example * Fix links in NameAI API readme (#579) * fix links in readme * Update README.md --------- Co-authored-by: kwrobel.eth <djstrong@gmail.com> * feat: [sc-26164] Finish Transition of alpha.namekit.io landing page to namekit.io (#563) * add ExploreNameGraphForm to Namegraph services section * Add namekit react Button to ExploreNameGraphForm * support empty input on ExploreNameGraphForm * use namekit react Input on explore-namegraph-form * improve button and text placement --------- Co-authored-by: lightwalker.eth <126201998+lightwalker-eth@users.noreply.github.com> * feat: [sc-26169] Refine namekit.io landing page (#564) * use namekit-react for X button * add Try the Alpha to mobile navigation link * avoid just 1 word wrapping to the next line on get-your-web3-name section * add "try now" button on hero section * fix button placement * fix text placement on hero section * fix screen limit * fix balancer generated layout shifts * add IconButton to mobile version of namekit.io --------- Co-authored-by: lightwalker.eth <126201998+lightwalker-eth@users.noreply.github.com> * Notrab/sc 26147/nameai fix ux of unnormalized label submissions (#575) * feat(nameai): turn off autoComplete * add data-1p-ignore * rename field * add client side validation and loading states * fix type issues * fix rank form errors * update tsconfig * fix type * sort page ux * feat: [SC-26223] ✨ Migrate NFTAvatar to namegraph.dev based on next/image dependency * feat: [SC-26223] 💄 Make NFTAvatar look shiny in the Ui * fix: [SC-26223] 🐛 Update DisplayedName to fix labels displaying for subdomains * fix: [SC-26223] 🐛 Clean unnecessary code and make it possible to switch suffixes and recharge avatars right away * fix: [SC-26223] 🐛 Fix atropos react typescript alignment * feat: [SC-26223] ✨ Remove file and optimize code syntax --------- Co-authored-by: Piotr Zawiślan <59446552+Byczong@users.noreply.github.com> Co-authored-by: kwrobel.eth <djstrong@gmail.com> Co-authored-by: Eduardo <eduramme@gmail.com> Co-authored-by: lightwalker.eth <126201998+lightwalker-eth@users.noreply.github.com> Co-authored-by: Jamie Barton <jamie@notrab.dev>
…bounced Input in collections page
…hub.com/namehash/namekit into francoaguzzi/sc-26202/name-details-page
There was a problem hiding this comment.
@FrancoAguzzi Nice! Some pretty huge upgrades here 🚀 Reviewed and shared feedback. Suggest that we merge this in right away and then work on the feedback in a separate future PR 👍 Thanks!
| @@ -43,10 +43,10 @@ export const CollectionCard = ({ | |||
| {collection.top_labels.map((tag) => ( | |||
| <Link | |||
| key={tag.label} | |||
| href={`/name/${buildENSName(tag.label.replace(" ", "")).name}`} | |||
| href={`/name/${buildENSName(tag.label).name}`} | |||
There was a problem hiding this comment.
Can you help me understand why we call buildENSName here? tag.label is guaranteed to always be normalized already.
You may need to do other operations here though:
- Append the current suffix to
tag.label, such that links to name detail pages are always "fully qualified". - URL Encode to ensure the link works even if
tag.labelhas special characters in it.
Please don't put all this logic inline and instead define a helper function that takes as input a tag and returns a string with the URL to that name in perfect form that's ready to use in any href field.
There was a problem hiding this comment.
@lightwalker-eth I've been trying for some time something that is unsuccessful so I will explain the approach that I took which is successful:
- It is almost impossible to align strings between: title, search bar and URL params. This behaves this way because we need to constantly deal with string splitting if we want these 3 aligned, which is painful once we should deal with searches like "franco.camelo". This example exposes how we cannot simply split the string in its first dot (.) and change its suffix. We rather need to always have clarity of what is the user search term and the selected suffix.
- The successful approach I built easily clarifies this separation: in the URL we have "search" param and "tld" param. In the title, we show both the search + tld. Finally, in the search bar we show only the search. This guarantees we have all customizations aligned while code does not need to handle lots of logics regarding where to split user's search term and neither needs to align three places with the same value at each new search.
Understanding that each one of the title, selected tld and search bar have its own responsibilities is a kick-start run out of the mental-challenge that results in having these three storing the same value (title = "franco.camelo.box" & search bar = "franco.camelo.box" & selected tld = "franco.camelo.box")
Summed to this, storing title = "franco.camelo.box" & search bar = "franco.camelo" & selected tld = ".box" saves us from lots of computation work in user's browser and in our codebase. Here is the exposure of this approach!
Do you agree with using this oriented approach?
| @@ -370,12 +364,12 @@ export const ExploreCollectionPage = ({ id }: { id: string }) => { | |||
| params.page | |||
| ]?.suggestions.map((suggestion) => ( | |||
| <Link | |||
| href={`/name/${getNormalizedAndTrimmedName(suggestion)}`} | |||
| href={`/name/${buildENSName(suggestion.label).name}`} | |||
There was a problem hiding this comment.
Please see my other comment about this.
| @@ -449,12 +443,12 @@ export const ExploreCollectionPage = ({ id }: { id: string }) => { | |||
| {sampledNameIdeas?.map((suggestion) => { | |||
| return ( | |||
| <Link | |||
| href={`/name/${getNormalizedAndTrimmedName(suggestion)}`} | |||
| href={`/name/${buildENSName(suggestion.label).name}`} | |||
There was a problem hiding this comment.
Please see my other comment about this.
| @@ -520,12 +514,12 @@ export const ExploreCollectionPage = ({ id }: { id: string }) => { | |||
| {scrambledNameIdeas?.map((suggestion) => { | |||
| return ( | |||
| <Link | |||
| href={`/name/${getNormalizedAndTrimmedName(suggestion)}`} | |||
| href={`/name/${buildENSName(suggestion.label).name}`} | |||
There was a problem hiding this comment.
Please see my other comment about this.
| @@ -331,6 +328,10 @@ export default function ExploreCollectionsPage() { | |||
| handleSearch(params.search); | |||
| }, [params.search]); | |||
|
|
|||
| const getLabelWithoutWhitespace = (label: string): string => { | |||
| return buildENSName(label.replace(" ", "")).name; | |||
There was a problem hiding this comment.
I think in this context we may need to call buildENSName since it is normalizing the raw search value.
| <Skeleton className="w-40 h-8" /> | ||
| <div className="max-w-7xl flex flex-col space-y-8 lg:space-y-0 lg:space-x-8 lg:flex-row mx-auto py-8 w-full"> | ||
| <div className="p-6 flex justify-start flex-col w-full"> | ||
| {NameWithDefaultSuffix({ name: label }) ? ( |
There was a problem hiding this comment.
Why default suffix? Shouldn't we use current?
| <div className="mx-auto"> | ||
| <NftAvatar | ||
| name={buildENSName( | ||
| NameWithDefaultSuffix({ name: label, reloadOnChange: true }), |
There was a problem hiding this comment.
Why default suffix? Shouldn't we use current?
| } | ||
| <Link | ||
| key={suggestion.label} | ||
| href={`/name/${suggestion.label}`} |
There was a problem hiding this comment.
We should have a helper function that's used across the whole app to ensure we get this detail right when generating these hrefs. This helper function should perform URL encoding of the label.
| ] | ||
| } | ||
| </SelectItem> | ||
| <NameWithDefaultSuffix name={suggestion.label} /> |
There was a problem hiding this comment.
Why default and not current?
| <div> | ||
| <div className="text-3xl font-semibold mb-4 break-all"> | ||
| {label ? ( | ||
| <NameWithDefaultSuffix name={label} /> |
There was a problem hiding this comment.
Why default and not current?
…features