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.

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

Jump to bottom

feat: premium servers list search bar #103

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

feat: premium servers list search bar #103

wants to merge 20 commits into from
+440 −297

Conversation

dpaulos6
Copy link
Collaborator

@dpaulos6 dpaulos6 commented Jan 7, 2025

No description provided.

@mezotv mezotv requested a review from Woofer21 January 7, 2025 17:44
@dpaulos6 dpaulos6 marked this pull request as ready for review January 7, 2025 19:35
Woofer21
Woofer21 requested changes Jan 8, 2025
View reviewed changes
Copy link
Collaborator

@Woofer21 Woofer21 left a comment

Choose a reason for hiding this comment

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

Looks good, though this will need to be re-written when #104 gets merged since this file gets changed in that PR.

app/premium/page.tsx Outdated Show resolved Hide resolved
@dpaulos6
Copy link
Collaborator Author

dpaulos6 commented Jan 8, 2025

Looks good, though this will need to be re-written when #104 gets merged since this file gets changed in that PR.

Thanks for letting me know! Will keep this in mind and make sure to rewrite as soon as #104 gets merged.

@mezotv
Copy link
Member

mezotv commented Jan 8, 2025

Just merged the other pr @dpaulos6 feel free to request review here again once possible conflicts are gone!

@dpaulos6 dpaulos6 requested a review from Woofer21 January 8, 2025 16:19
Woofer21
Woofer21 requested changes Jan 8, 2025
View reviewed changes
Copy link
Collaborator

@Woofer21 Woofer21 left a comment

Choose a reason for hiding this comment

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

In addition to the below changes, it would also be nice to get a loading state for the while the server list is loading.

components/Premium/Content.tsx Outdated Show resolved Hide resolved
components/Premium/Content.tsx Outdated Show resolved Hide resolved
components/Premium/Content.tsx Outdated Show resolved Hide resolved
components/Premium/Content.tsx Outdated Show resolved Hide resolved
components/Premium/Content.tsx Outdated Show resolved Hide resolved
Woofer21
Woofer21 requested changes Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@Woofer21 Woofer21 left a comment

Choose a reason for hiding this comment

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

Pretty good, 2 things:

  1. Page crashes when you try to navigate to it while logged out
  2. Lets added a loading state & disable the button after you press it & its waiting for the stripe page to open

Woofer21 and others added 4 commits January 9, 2025 09:57
Woofer21
Woofer21 requested changes Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@Woofer21 Woofer21 left a comment

Choose a reason for hiding this comment

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

Few changes to loading state, otherwise looking good.

app/premium/_components/checkoutButton.tsx Outdated Show resolved Hide resolved
app/premium/_components/checkoutButton.tsx Outdated Show resolved Hide resolved
components/Premium/Content.tsx Outdated Show resolved Hide resolved
icons/loading.tsx Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Woofer21 Woofer21 Woofer21 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dpaulos6 @mezotv @Woofer21