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 leaderboard FE #254

Merged
merged 23 commits into from
Nov 15, 2023
Merged

feat: add leaderboard FE #254

merged 23 commits into from
Nov 15, 2023

Conversation

ayushtom
Copy link
Contributor

@ayushtom ayushtom commented Oct 17, 2023

closes #250
closes #222
closes #251

@vercel
Copy link

vercel bot commented Oct 17, 2023

@ayushtom is attempting to deploy a commit to the StarknetID Team on Vercel.

To accomplish this, @ayushtom needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

@ayushtom ayushtom changed the title feat: add base UI and mobile view feat: add leaderboard FE Oct 17, 2023
@ayushtom ayushtom added the 🚧 In progress do not merge Pull Request in progress, please do not merge label Oct 17, 2023
@ayushtom ayushtom force-pushed the ayush/leaderboard-fe branch from b7bc89c to 79ea66a Compare November 5, 2023 13:34
@ayushtom ayushtom marked this pull request as ready for review November 6, 2023 22:18
@ayushtom ayushtom added 🔥 Ready for review This pull request needs a review and removed 🚧 In progress do not merge Pull Request in progress, please do not merge labels Nov 7, 2023
@ayushtom
Copy link
Contributor Author

ayushtom commented Nov 7, 2023

I noticed that i have used tailwind classes. Will replace them with css styling in next commit

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.

Super cool ! Some comments and questions

components/UI/ChipList.tsx Outdated Show resolved Hide resolved
components/UI/Divider.tsx Outdated Show resolved Hide resolved
components/UI/RankCards.tsx Outdated Show resolved Hide resolved
components/UI/RankCards.tsx Outdated Show resolved Hide resolved
services/apiService.ts Show resolved Hide resolved
services/apiService.ts Outdated Show resolved Hide resolved
utils/numberService.ts Show resolved Hide resolved
utils/domainService.ts Show resolved Hide resolved
@fricoben fricoben added ❌ Change request Change requested from reviewer and removed 🔥 Ready for review This pull request needs a review labels Nov 7, 2023
Copy link
Collaborator

@Kevils Kevils left a comment

Choose a reason for hiding this comment

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

1 - Can we add an hover state/animation ?
image

1 - Add top padding on mobile like 24px
Capture d’écran 2023-11-12 à 11 10 28

1 - Is it bug ?
Capture d’écran 2023-11-12 à 11 12 19

1 - Does the period filter work?
https://github.com/starknet-id/starknet.quest/assets/144677881/8760e8d6-b1c5-45c3-898f-0dd3670ed08c

1 - I believe you can only perform a search by typing the user's name exactly.
Is it possible to display the list letter by letter? Something like that
image

Thanks ! looks very good

@ayushtom
Copy link
Contributor Author

1 - Does the period filter work?

yes. The data is updated 2 days back all at once. So the date is filter can not filter on 7 days yet. Will take some time for data to have different timestamps.

1 - I believe you can only perform a search by typing the user's name exactly.
This will need another API endpoint. Me and thomas discussed and think this might be a bit expensive. We can try this for next version update on leaderboard

@ayushtom
Copy link
Contributor Author

Design and code is reviewed and approved.

Copy link
Member

@Th0rgal Th0rgal left a comment

Choose a reason for hiding this comment

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

lgtm

@Th0rgal Th0rgal merged commit 3c673f1 into testnet Nov 15, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🦾 Needs front-end review This pull request needs a review from designer 🔥 Ready for review This pull request needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add leaderboard search feature Connecting leaderboard to backend Leaderboard Front-end implementation
5 participants