Skip to content

feat(namekit-react): Identity #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
notrab wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat(namekit-react): Identity #439

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db46f78
feat(namekit-react): Identity
notrab Oct 15, 2024
450998f
wrap the client component
notrab Oct 15, 2024
1d92fb1
feat: handle avatar and nameguard report state
notrab Oct 22, 2024
105a487
feat: add ens profile link
notrab Oct 22, 2024
ef9dbc5
feat: follower count
notrab Oct 23, 2024
3ddd190
rename ethfollow func
notrab Oct 28, 2024
ec2cb9c
feat: apply feedback from lightwalker
notrab Nov 3, 2024
752ac3a
updates to profile link
notrab Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 128 additions & 10 deletions apps/storybook.namekit.io/stories/Namekit/Identity.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from "react";
import type { Meta, StoryObj } from "@storybook/react";
import { Identity } from "@namehash/namekit-react/client";
import { Identity, NameKitProvider } from "@namehash/namekit-react/client";

const meta: Meta<typeof Identity.Root> = {
title: "Namekit/Identity",
Expand All @@ -21,7 +21,7 @@ export default meta;

type Story = StoryObj<typeof Identity.Root>;

const IdentityCard: React.FC<{
const DefaultIdentityCard: React.FC<{
address: string;
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check all places in the code where you have an address param. These should all use the Address type defined in viem.

network?: "mainnet" | "sepolia";
returnNameGuardReport?: boolean;
Expand All @@ -35,29 +35,147 @@ const IdentityCard: React.FC<{
<Identity.Name />
<Identity.Address />
<Identity.NameGuardShield />
<Identity.ENSProfileLink />
<Identity.ProfileLink>
<div>
<ENSLogo />
<span>View on ENS App</span>
</div>
</Identity.ProfileLink>
<Identity.Followers />
</Identity.Root>
);

const customAppConfig = {
profileLinks: {
getProfileURL: (address: string) => `/profiles/${address}`,
getProfileLink: (address: string, children: React.ReactNode) => (
<a href={`/profiles/${address}`}>{children}</a>
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems nice to implement this example such that it calls getProfileURL.

Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any special reason not to use our <Link in this example?

Copy link
Member Author

@notrab notrab Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as before.

),
},
};

const CustomAppIdentityCard: React.FC<{ address: string }> = ({ address }) => (
<NameKitProvider config={customAppConfig}>
<Identity.Root address={address}>
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest we make a few updates here to put our "Identity" components on a stronger foundation. Specifically: an "identity reference" is more than just an address. It is a combination of a chain and an address.

  1. Suggest defining an interface for ImplicitIdentityReference that combines both an address and a chain, but where the chain param is optional. The purpose of this interface is to optimize DX for the average case where all addresses within an app are on the same chain. It should be noted how this isn't always the case though, and increasingly won't be in the future the way things are headed.
  2. Suggest defining an interface for IdentityReference that extends ImplicitIdentityReference but makes the chain param required.
  3. Suggest defining a little utility function that can be used within the context of a NameKitProvider that automatically converts any ImplicitIdentityReference to an explicit IdentityReference using the configured chain as the default.
  4. There's a bunch of functions here that currently only take an "address" param. These should change so that they instead take a param of type IdentityReference or ImplicitIdentityReference. Inside the implementation of these functions, we can call the utility function described above that takes either an IdentityReference or ImplicitIdentityReference as a param and always returns an explicit IdentityReference for use in all our internal logic.
  5. Functions such as getProfileURL and getProfileLink should take an IdentityReference as a param. The default implementation of these functions should then consider the chain parameter and not only the address. For example. If the chain param is mainnet, then our existing strategy is good. If the chain param is anything else (such as sepolia, etc..), then there isn't an existing profile service we can easily link to for this. So instead suggest making our default implementation of all the profile link generator functions trigger some alert box or something like that for the case that the chain is anything but mainnet. Once we build the backend APIs into NameKit for generating profile data, we can come back to this and implement some improved default behaviour for showing profiles on chains other than mainnet.

Here's an acceptance criteria for everything described above: If I use our Identity components to show the identity for a chain other than mainnet (such as sepolia) I shouldn't get a profile link to view the profile of that address on mainnet as that wouldn't actually be the correct profile / identity.

Please ask me if any questions 👍

<Identity.Avatar />
<Identity.Name />
<Identity.ProfileLink>
<button>View Profile</button>
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any special reason not to use our Button?

Copy link
Member Author

@notrab notrab Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned before, I'm avoiding the UI side of things until we nail the DX. Swapping out components is easy, but if we need to make changes to the UI components, it could cause conflicts so I've opted not to focus on that yet.

</Identity.ProfileLink>
</Identity.Root>
</NameKitProvider>
);

const modalConfig = {
profileLinks: {
getProfileURL: (address: string) => `#${address}`,
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's document the goals / responsibilities of this getProfileURL function. For example, I'm a bit worried if we have it return just an anchor link like this, because that might work on some pages of an app, but not others depending on how the app is implemented.

It will help if we could document the responsibility / goal of the getProfileURL function. Perhaps we should make it so that it must always return a fully qualified URL for the profile and never a relative URL or just a relative anchor link like this?

getProfileLink: (address: string, children: React.ReactNode) => (
<button
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any special reason not to use our Button here?

onClick={(e) => {
e.preventDefault();
alert(`Would open modal for ${address}`);
}}
>
{children}
</button>
),
},
};

const ModalIdentityCard: React.FC<{ address: string }> = ({ address }) => (
<NameKitProvider config={modalConfig}>
<Identity.Root address={address}>
<Identity.Avatar />
<Identity.Name />
<Identity.ProfileLink>
<>
<span>Open Profile Modal</span>
<span>→</span>
</>
</Identity.ProfileLink>
</Identity.Root>
</NameKitProvider>
);

export const Default: Story = {
args: {
address: "0x838aD0EAE54F99F1926dA7C3b6bFbF617389B4D9",
network: "mainnet",
className: "rounded-xl",
},
render: (args) => <IdentityCard {...args} />,
render: (args) => <DefaultIdentityCard {...args} />,
};

export const MultipleCards: Story = {
render: () => (
<>
<IdentityCard address="0x838aD0EAE54F99F1926dA7C3b6bFbF617389B4D9" />
<IdentityCard address="0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045" />
<IdentityCard
address="0xf81bc66316a3f2a60adc258f97f61dfcbdd23bb1"
returnNameGuardReport
/>
<DefaultIdentityCard address="0x838aD0EAE54F99F1926dA7C3b6bFbF617389B4D9" />
<CustomAppIdentityCard address="0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045" />
<ModalIdentityCard address="0xf81bc66316a3f2a60adc258f97f61dfcbdd23bb1" />
</>
),
};

export const ProfileLinkVariants: Story = {
render: () => {
const address = "0x838aD0EAE54F99F1926dA7C3b6bFbF617389B4D9";

return (
<div>
<div>
<h3>Default ENS App Link</h3>
<Identity.Root address={address}>
<Identity.ProfileLink>
<div className="nk-flex nk-items-center nk-gap-2">
<ENSLogo />
<span>View on ENS App</span>
</div>
</Identity.ProfileLink>
</Identity.Root>
</div>

<div>
<h3>Custom App Link</h3>
<NameKitProvider config={customAppConfig}>
<Identity.Root address={address}>
<Identity.ProfileLink>
<button>View Profile</button>
</Identity.ProfileLink>
</Identity.Root>
</NameKitProvider>
</div>

<div>
<h3>Modal Trigger</h3>
<NameKitProvider config={modalConfig}>
<Identity.Root address={address}>
<Identity.ProfileLink>
<>
<span>Open Profile Modal</span>
<span>→</span>
</>
</Identity.ProfileLink>
</Identity.Root>
</NameKitProvider>
</div>
</div>
);
},
};

const ENSLogo = () => (
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest we move this into a dedicated UI component inside NameKit React.

<svg
fill="none"
height="16"
viewBox="0 0 202 231"
width="14"
xmlns="http://www.w3.org/2000/svg"
>
<g fill="#0080bc">
<path d="m98.3592 2.80337-63.5239 104.52363c-.4982.82-1.6556.911-2.2736.178-5.5924-6.641-26.42692-34.89-.6463-60.6377 23.5249-23.4947 53.4891-40.24601 64.5942-46.035595 1.2599-.656858 2.587.758365 1.8496 1.971665z" />
<path d="m94.8459 230.385c1.2678.888 2.8299-.626 1.9802-1.918-14.1887-21.581-61.3548-93.386-67.8702-104.165-6.4264-10.632-19.06614-28.301-20.12056-43.4178-.10524-1.5091-2.19202-1.8155-2.71696-.3963-.8466 2.2888-1.74793 5.0206-2.58796 8.1413-10.60469 39.3938 4.79656 81.1968 38.24488 104.6088l53.0706 37.148z" />
<path d="m103.571 228.526 63.524-104.523c.498-.82 1.656-.911 2.274-.178 5.592 6.64 26.427 34.89.646 60.638-23.525 23.494-53.489 40.246-64.594 46.035-1.26.657-2.587-.758-1.85-1.972z" />
<path d="m107.154.930762c-1.268-.8873666-2.83.625938-1.98 1.918258 14.189 21.58108 61.355 93.38638 67.87 104.16498 6.427 10.632 19.066 28.301 20.121 43.418.105 1.509 2.192 1.815 2.717.396.846-2.289 1.748-5.02 2.588-8.141 10.604-39.394-4.797-81.1965-38.245-104.609z" />
</g>
</svg>
);
2 changes: 1 addition & 1 deletion packages/namekit-react/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ export {
} from "./components/CurrencySymbol/CurrencySymbol";
export { TruncatedText } from "./components/TruncatedText";

export { Identity } from "./components/Identity";
export { Identity, NameKitProvider } from "./components/Identity";
93 changes: 58 additions & 35 deletions packages/namekit-react/src/components/Identity.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,34 @@ import {
Network,
type SecurePrimaryNameResult,
} from "@namehash/nameguard";
import cc from "classcat";

export interface ProfileLinkConfig {
getProfileURL: (address: string) => string;
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few ideas here:

  1. Please see other comments I've shared about this getProfileUrl function.
  2. Maybe we change getProfileURL so that it can return either a Url (rather than a string) or null for the case that an app doesn't have a dedicated URL for the profile of a particular identity reference? This could be because the app either never renders profiles or only renders profiles in modals. It also could be because a particular "Identity Reference" is on a chain where the app doesn't support rendered profiles for. For example, the ENS App only renders profiles for Ethereum Mainnet. If you're trying to render the profile for an address on a different chain, such as Sepolia testnet, there's currently no elegant service to redirect someone to. So an app might want to say.. if the identity reference is for a chain I don't support, then I conditionally don't have any profile URL for that identity reference.

Copy link
Member Author

@notrab notrab Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lightwalker-eth I had another idea to support this. What if we did something like this:

interface URLGenerationStrategy {
  generateURL(address: string, network: string): URL | null;
}

export class ProfileLinkGenerator {
  constructor(
    private name: string,
    private strategy: URLGenerationStrategy
  ) {}

  getName(): string {
    return this.name;
  }

  getProfileURL(address: string, network: string): URL | null {
    return this.strategy.generateURL(address, network);
  }
}

Then the usage would be:

const ensStrategy = {
  generateURL: (address: string, network: string) => 
    network === 'mainnet' ? new URL(`https://app.ens.domains/${address}`) : null
};

const ENSProfileLink = new ProfileLinkGenerator("ENS", ensStrategy);

getProfileLink: (address: string, children: ReactNode) => JSX.Element;
}
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I shared some other comments in this PR with the idea of having a default fallback implementation of the profile link anchor. Ex: An ENS Logo, etc..

It seems to me that one nice way to do this would be to define another function in this interface, for example: getProfileLinkAnchor that takes no params and returns a JSX.Element. We could have a default implementation that provides an anchor that is an ENS Logo, for example.

Appreciate your advice / suggestions 👍


interface NameKitConfig {
profileLinks?: ProfileLinkConfig;
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest we make it so that this field is always defined. If no custom generator was provided, then we should make it equal to the default generator we define.

}

const NameKitConfigContext = createContext<NameKitConfig>({});

export const NameKitProvider: React.FC<{
children: React.ReactNode;
config: NameKitConfig;
}> = ({ children, config }) => {
return (
<NameKitConfigContext.Provider value={config}>
{children}
</NameKitConfigContext.Provider>
);
};

export const useNameKitConfig = () => useContext(NameKitConfigContext);

interface IdentityContextType {
network: string;
network: Network;
address: string;
returnNameGuardReport: boolean;
loadingState: "loading" | "error" | "success";
Expand Down Expand Up @@ -243,42 +268,40 @@ const Followers = ({ className, ...props }: SubComponentProps) => {
);
};

const ENSLogo = () => (
<svg
fill="none"
height="16"
viewBox="0 0 202 231"
width="14"
xmlns="http://www.w3.org/2000/svg"
>
<g fill="#0080bc">
<path d="m98.3592 2.80337-63.5239 104.52363c-.4982.82-1.6556.911-2.2736.178-5.5924-6.641-26.42692-34.89-.6463-60.6377 23.5249-23.4947 53.4891-40.24601 64.5942-46.035595 1.2599-.656858 2.587.758365 1.8496 1.971665z" />
<path d="m94.8459 230.385c1.2678.888 2.8299-.626 1.9802-1.918-14.1887-21.581-61.3548-93.386-67.8702-104.165-6.4264-10.632-19.06614-28.301-20.12056-43.4178-.10524-1.5091-2.19202-1.8155-2.71696-.3963-.8466 2.2888-1.74793 5.0206-2.58796 8.1413-10.60469 39.3938 4.79656 81.1968 38.24488 104.6088l53.0706 37.148z" />
<path d="m103.571 228.526 63.524-104.523c.498-.82 1.656-.911 2.274-.178 5.592 6.64 26.427 34.89.646 60.638-23.525 23.494-53.489 40.246-64.594 46.035-1.26.657-2.587-.758-1.85-1.972z" />
<path d="m107.154.930762c-1.268-.8873666-2.83.625938-1.98 1.918258 14.189 21.58108 61.355 93.38638 67.87 104.16498 6.427 10.632 19.066 28.301 20.121 43.418.105 1.509 2.192 1.815 2.717.396.846-2.289 1.748-5.02 2.588-8.141 10.604-39.394-4.797-81.1965-38.245-104.609z" />
</g>
</svg>
);

const ENSProfileLink = ({ className, ...props }: SubComponentProps) => {
const { identityData, loadingState } = useIdentity();

if (loadingState !== "success" || !identityData?.display_name) {
interface ProfileLinkProps extends SubComponentProps {
config?: ProfileLinkConfig;
}

const ProfileLink = ({
className,
children,
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to make children an optional param, and if not provided, to have some default children that would for example just be the ENS Logo?

config: instanceConfig,
...props
}: ProfileLinkProps) => {
const { loadingState, address } = useIdentity();
const globalConfig = useNameKitConfig();

if (loadingState !== "success") {
return null;
}

return (
<a
href={`https://app.ens.domains/${identityData.display_name}`}
target="_blank"
rel="noopener noreferrer"
className={`namekit-ens-profile-link ${className}`}
{...props}
>
<ENSLogo />
<span className="nk-ml-1">ENS Profile</span>
</a>
);
const config = instanceConfig ||
globalConfig.profileLinks || {
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest extracting some of the ideas here out into a named / exported class that provides a default implementation of ProfileLinkGenerator / ProfileLinkConfig.

One thing that can be nice about this: Someone could create a subclass of the default ProfileLinkGenerator implementation and only override the getProfileURL function in it and all the other defaults can continue to work.

getProfileURL: (address) => `https://app.ens.domains/${address}`,
getProfileLink: (address, children) => (
<a
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use our Link here?

href={`https://app.ens.domains/${address}`}
Copy link
Member

@lightwalker-eth lightwalker-eth Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems nice to implement this in terms of getProfileURL?

target="_blank"
rel="noopener noreferrer"
className={cc(["namekit-profile-link", className])}
{...props}
>
{children}
</a>
),
};

return config.getProfileLink(address, children);
};

export const Identity = {
Expand All @@ -287,6 +310,6 @@ export const Identity = {
Name,
Address,
NameGuardShield,
ENSProfileLink,
ProfileLink,
Followers,
};