Skip to content
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.

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

[WIP] Add profile page #3

Merged
merged 14 commits into from
Jan 23, 2023
Merged

Conversation

irisdv
Copy link
Collaborator

@irisdv irisdv commented Jan 4, 2023

close #2

@vercel
Copy link

vercel bot commented Jan 4, 2023

@irisdv is attempting to deploy a commit to the Starknet Builders Team on Vercel.

A member of the Team first needs to authorize it.

};

const Activity: FunctionComponent<ActivityData> = (props) => {
const { type, description, timestamp } = props;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tu peux directement mettre ça comme ça :

const Activity: FunctionComponent<ActivityData> = ({ type, description, timestamp }) => {

import DiscordIcon from "../../components/icons/discordIcon";
import styles from "../../styles/Profile.module.css";

type ClickableDiscordIconProps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Tu répètes ce type dans plusieurs fichiers. Il a y plusieurs solutions pour éviter la répétition ici mais nous ce qu'on fait (c'est le plus simple meme si c'est pas le plus organisé) c'est de créer un fichier type.d.ts ou tu met les types que tu vas réutiliser plusieurs fois.

Après en l'occurence je trouve que ça fait plus propre d'avoir le type dans le fichier ici qu'en penses tu ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je pense que ce serait mieux d'avoir les types dans un fichier séparé mais comme dans app.starknet.id vous les avez répété dans chaque fichier j'ai suivi la même convention :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok ne répétons pas mes erreurs alors go dans un fichier séparés. Je vais le changer dans app aussi.

@@ -0,0 +1,23 @@
import React, { FunctionComponent } from "react";

export type IconProps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Pareil pour le type ;)

import React, { FunctionComponent, useEffect, useState } from "react";
import styles from "../styles/Soulbound.module.css";

export type SoulBoundData = {
Copy link
Contributor

@fricoben fricoben Jan 9, 2023

Choose a reason for hiding this comment

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

SoulBound ==> Soulbound (ce n'est que 1 seul mot donc le camel case se fait comme ça je pense).

Au niveau du naming j'aurais dis aussi SoulBoundData ==> SoulboundProps pour unifier avec le reste de nos composants

process.env.NEXT_PUBLIC_NAMING_CONTRACT as string,
new Provider({
sequencer: {
network: "goerli-alpha", // "mainnet-alpha"
Copy link
Contributor

@fricoben fricoben Jan 9, 2023

Choose a reason for hiding this comment

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

On changera dans le futur ça c'est que pour tes tests c'est bien ça ? Parce que le profil sera uniquement dispo en mainnet je pense 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oui c'est pour pouvoir tester de récupérer les données du verifier. Je peux ajouter une variable d'environnement sinon

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok génial

pages/[id].tsx Outdated
)
.then((response) => response.json())
.then((data: Identity) => {
console.log("data fetched", data);
Copy link
Contributor

Choose a reason for hiding this comment

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

un log a enlevé 👍

pages/[id].tsx Outdated
},
];

const activityTest = [
Copy link
Contributor

Choose a reason for hiding this comment

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

A enlevé dans la PR final aussi

pages/[id].tsx Outdated
}
>
{" "}
NFTs & Soulbound tokens
Copy link
Contributor

Choose a reason for hiding this comment

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

Pour l'instant juste Souldbounds tokens finalement 👍

className={styles.SbtCard}
onMouseEnter={() => setShowInfo(true)}
onMouseLeave={() => setShowInfo(false)}
onClick={() => console.log("open")}
Copy link
Contributor

Choose a reason for hiding this comment

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

Rien onClick pour le moment je pense, ce log est donc à delete 👍

pages/[id].tsx Outdated
@@ -0,0 +1,220 @@
import { NextPage } from "next";
Copy link
Contributor

Choose a reason for hiding this comment

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

Le naming du fichier pourrait etre plus expressif pour exprimer la possibilité de mettre le .stark (ce qui est pas le cas dans notre /identities sur app.starknet.id pour le moment).

idOrUsername un truc du genre. Tu en penses quoi ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oui c'est mieux, je change!

Copy link
Contributor

@fricoben fricoben left a comment

Choose a reason for hiding this comment

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

Très clean ! Quelques petits changements et on fait l'api aspect puis c'est good !

Copy link
Contributor

@fricoben fricoben left a comment

Choose a reason for hiding this comment

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

Quelques petits détails mais on avance bien c'est cool.

}
}, [tokenId]);

// useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Pq c'est commenté ? Il y avait un pb ?

addr,
}) => {
const copyToClipboard = () => {
navigator.clipboard.writeText(addr as string);
Copy link
Contributor

Choose a reason for hiding this comment

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

Pas besoin du as string ici non ? Vu que tu as typé ClickableChainIconProps

<p>
{addr.substring(0, 6) +
"..." +
addr.substring(addr.length - 4, addr.length)}
Copy link
Contributor

Choose a reason for hiding this comment

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

On a une fonction testée sur l'app (pas besoin de la tester ici) pour ça ici que tu peux réutiliser.

export function minifyAddress(address: string): string {
  const firstPart = address.substring(0, 4);
  const secondPart = address.substring(address.length - 3, address.length);

  return (firstPart + "..." + secondPart).toLowerCase();
}


useEffect(() => {
if (imageUri.toLowerCase().startsWith("ipfs://")) {
setUrl(imageUri.replace("ipfs://", "https://ipfs.io/ipfs/"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Bien vu ça 👍

constants.ts Outdated
@@ -0,0 +1,52 @@
export const activityContractsTestnet = {
JediSwap: "0x2bcc885342ebbcbcd170ae6cafa8a4bed22bb993479f49806e72d96af94c965",
Copy link
Contributor

Choose a reason for hiding this comment

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

a placer dans utils je pense ou autre part que dans la racine du dossier non ?

return domain.endsWith(".stark");
}

export function minifyAddressOrDomain(
Copy link
Contributor

Choose a reason for hiding this comment

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

je viens de faire une PR pour changer cette fonction minifyAddressOrDomain utils qui est nul et faire des trucs plus clean.

https://github.com/starknet-id/app.starknet.id/pull/87/files

@vercel
Copy link

vercel bot commented Jan 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
starknet-id ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 23, 2023 at 6:42PM (UTC)

@Th0rgal
Copy link
Member

Th0rgal commented Jan 13, 2023

image
erreur de build apparemment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Faire une page profil sous la forme starknet.id/name.stark
3 participants