✨ Integrate NameGraph SDK API call into Explore Collections page and make collections exploring page work #499
Conversation
…github.com/namehash/namekit into francoaguzzi/sc-25948/hotfix-organize-code
…/hotfix-organize-code
FrancoAguzzi
requested review from
djstrong,
Carbon225,
notrab,
lightwalker-eth,
BanaSeba and
eduramme
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
|
…ections page and make collections exploring page work
FrancoAguzzi
changed the title
There was a problem hiding this comment.
@FrancoAguzzi Hey, appreciate the updates here. Exciting to see. I've approved this PR so that you can merge it, but please carefully review my feedback and either resolve it in this PR, or create a separate PR that resolves it. Thank you!
| "./pages/**/*.{js,ts,jsx,tsx,mdx}", | ||
| "./components/**/*.{js,ts,jsx,tsx,mdx}", | ||
| "./app/**/*.{js,ts,jsx,tsx,mdx}", | ||
| ], | ||
| safelist: [ |
There was a problem hiding this comment.
Can you help me understand the goal here?
There was a problem hiding this comment.
Yes! Safelist makes it possible to generate programatically the bg colors used by the name ideas pills.
It seems like Tailwind runs in a different "time-space" (😅) than programatic JS, which can be attested by removing the "safelist" from Tailwind config: when we do so, the bg-[#SOMECOLOR] is not applied. Safelisting in Tailwind means including this class in Tailwind's bundle upfront, guaranteeing the classes inside "safelist" array will have corresponding Tailwind styling config present in the CSS bundle. 🙂 Does this help?
apps/namegraph.dev/components/mini-apps/explore-collections/suggestion-category-header.tsx
Outdated
Show resolved
Hide resolved
...amegraph.dev/components/mini-apps/explore-collections/recursive-related-collection-pills.tsx
Outdated
Show resolved
Hide resolved
apps/namegraph.dev/lib/utils.ts
Outdated
| } | ||
|
|
||
| // This array is used for checking when a category is not a "related" category | ||
| const NameGraphSuggestionCategoryTypes = Object.values( |
There was a problem hiding this comment.
Seems this idea should be moved into the SDK?
There was a problem hiding this comment.
As this is a representation of the same NameGraph SDK present utility, but, with a different data structure, this is very much related to clients. I say this because apps might want to use this is different ways and mapping from one data structure to another one may be a responsibility of the client side, I think, so I wouldn't migrate this, what do you think?
apps/namegraph.dev/lib/utils.ts
Outdated
| export const generateNamesByQuery = async ( | ||
| input: LabelAndModeQueryParams, | ||
| ): Promise<NameGraphSuggestion[]> => { | ||
| if (input.label.includes(".")) |
There was a problem hiding this comment.
Why are we using this strategy? Why aren't we implementing the same strategy as before where if the input includes "." that we internally process that so as to only use the childmost label for our query?
Here we should identify back to the user the exact label that we're using to generate ideas with. Please note the related UX for this that we implemented here: https://www.namerank.io/?name=example.name.with.multiple.labels
- The users raw input query, which may include any number of labels.
- We show back to the user the label we're actually going to use in the query.
- We show the result of the query.
Please study the UX and implementation referenced above. We should follow the same pattern here.
There was a problem hiding this comment.
Now following the same pattern of using the childmost label for search. Mini-app is also displaying both the complete and the query-used strings, alongside with the results!
FrancoAguzzi
deleted the
francoaguzzi/sc-25978/new-namegraph-mini-app-2
branch
No description provided.