feat(ensadmin): ENS Explorer - explore names#1125
Conversation
…ing that 'actions' are present only for the 'Name Detail Page'
…avigating to "Name Detail" pages.
🦋 Changeset detectedLatest commit: d48eeed The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Y3drk Hey cool 👍 Reviewed and shared a few suggestions
…/feat/ensadmin/ens-explorer-explore-names
…/feat/ensadmin/ens-explorer-explore-names
…/feat/ensadmin/ens-explorer-explore-names
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Y3drk Hey nice work here 👍 I've pushed a few commits with refinements will merge this PR now. I've also added comments on this PR that contain background info that I think you'll find helpful for the future 😄
| export default function ExploreNamesPage() { | ||
| const router = useRouter(); | ||
| const [searchedName, setSearchedName] = useState<Name>(""); | ||
| const [rawInputName, setRawInputName] = useState<Name>(""); |
There was a problem hiding this comment.
@Y3drk Background info: We frequently need to separate raw input values from validated values. Whenever we are in a context like this suggest to explicitly name raw input values as "raw..." to avoid mistakes when using such raw unvalidated values.
| const formData = new FormData(e.target as HTMLFormElement); | ||
| const name = formData.get("ens-name") as Name; | ||
| // TODO: Input validation and normalization. | ||
| // see: https://github.com/namehash/ensnode/issues/1140 |
There was a problem hiding this comment.
@Y3drk I created this follow up issue with the detailed requirements.
| className="max-sm:self-stretch" | ||
| > | ||
| View Name | ||
| View Profile |
There was a problem hiding this comment.
@Y3drk Background info: We're exploring names, but technically we are viewing the profile for a name, not just the name itself.
| className="font-mono rounded-full" | ||
| asChild | ||
| > | ||
| <NameDisplay name={exampleName} /> |
There was a problem hiding this comment.
@Y3drk Background info: Whenever we display a name it should be done through a NameDisplay. I have a pending task to add special logic into NameDisplay for some best practices. It's a long story but please follow this.
| > | ||
| {exampleName} | ||
| </Button> | ||
| <NameLink name={exampleName} key={`example-name-link-${exampleName}`}> |
There was a problem hiding this comment.
@Y3drk Background info:
- These may be styled as buttons, but they should operate as links. Ex: I should be able to ctrl+click to open in a new tab or copy the link, etc.
- Whenever we link to a name we should use
NameLink
| return ( | ||
| <Link | ||
| href={nameDetailsRelativePath} | ||
| className={`inline-flex items-center gap-2 text-blue-600 hover:underline ${className || ""}`} |
There was a problem hiding this comment.
@Y3drk Background info: NameLink should just purely hold link logic, not influence stylings.
| <NameLink name={registration.name}> | ||
| <NameLink | ||
| name={registration.name} | ||
| className="inline-flex items-center gap-2 text-blue-600 hover:underline" |
There was a problem hiding this comment.
@Y3drk If we want a particular NameLink to have a particular style, we can achieve that goal like this.
| export function NameLink({ name, className, children }: PropsWithChildren<NameLinkProps>) { | ||
| const nameDetailsRelativePath = getNameDetailsRelativePath(name); | ||
| const { retainCurrentRawConnectionUrlParam } = useRawConnectionUrlParam(); | ||
| const href = retainCurrentRawConnectionUrlParam(getNameDetailsRelativePath(name)); |
There was a problem hiding this comment.
@Y3drk Background info: We shouldn't cause the selected connection to reset to the default selected connection whenever someone clicks on a link.
All our links need to retain the current selected connection. Perhaps there's a more elegant way to solve this issue, but in this PR I applied a simple strategy for achieving this goal.
Closes Issue #1091 . More details there.