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

feat: add achievements page #130

Merged
merged 15 commits into from
Sep 5, 2023
Merged

feat: add achievements page #130

merged 15 commits into from
Sep 5, 2023

Conversation

irisdv
Copy link
Collaborator

@irisdv irisdv commented Aug 22, 2023

Closes #126

@vercel
Copy link

vercel bot commented Aug 22, 2023

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

Name Status Preview Comments Updated (UTC)
goerli-starknet-quest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 1, 2023 4:28pm
starknet-quest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 1, 2023 4:28pm

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.

Some small changes.

UI feedback:
Capture d’écran 2023-08-29 à 12 14 21
Capture d’écran 2023-08-29 à 12 18 50

On the Braavos achievements I have bad wording and infos.

components/UI/navbar.tsx Show resolved Hide resolved
setHasChecked(true);
});
}
}, [userAchievements, address, hasChecked]);
Copy link
Contributor

Choose a reason for hiding this comment

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


const Footer: FunctionComponent = () => {
return (
<div className="relative">
<footer className={styles.footer}>
Powered by&nbsp;<strong>Starknet</strong>
<Link href="/partnership">Partnership</Link>
Copy link
Contributor

Choose a reason for hiding this comment

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

I can not see this component anywhere, this is normal right ? I prefer this to be hidden atm as I told you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There was already a footer that existed in the code that was not shown anywhere, I started updating it but I can revert this changes.

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.

Good ! Some comments :)

index % 2 === 0 ? "right center" : "left center"
}`,
backgroundSize: "30%",
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth putting that in a const before the return

achievement: AchievementDocument;
};

const CustomTooltip = styled(({ className, ...props }: TooltipProps) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a component in /UI

<div className={styles.headerContent}>
<h1 className={styles.title}>Achievements</h1>
<p className={styles.subtitle}>
Earn achievements to unlock lands and showcase them on your profile!
Copy link
Contributor

Choose a reason for hiding this comment

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

Earn achievements to unlock lands and showcase them on your profile! ==> Complete achievements and grow your Starknet on-chain reputation

Copy link
Member

Choose a reason for hiding this comment

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

why not using webp?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because it doesn't work as a background image

public/achievements/braavos/braavos_3.png Outdated Show resolved Hide resolved
pages/achievements.tsx Outdated Show resolved Hide resolved
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.

Some questions and comments 👏

components/UI/navbar.tsx Outdated Show resolved Hide resolved
components/achievements/level.tsx Outdated Show resolved Hide resolved
pages/achievements.tsx Show resolved Hide resolved
pages/achievements.tsx Show resolved Hide resolved
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.

Lgtm

pages/achievements.tsx Show resolved Hide resolved
@Th0rgal Th0rgal self-requested a review September 5, 2023 16:39
@fricoben fricoben merged commit 536a04d into testnet Sep 5, 2023
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.

Create the achivement page
3 participants