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

Fixed Joystream Pioneer Forum Back Link #3768 #4147

Merged
merged 6 commits into from
Mar 3, 2023
Merged

Fixed Joystream Pioneer Forum Back Link #3768 #4147

merged 6 commits into from
Mar 3, 2023

Conversation

vrrayz
Copy link
Contributor

@vrrayz vrrayz commented Feb 6, 2023

Fixed forum back link issue on #3768 by modifying the Pagination component to add a page query string and using a custom hook to check for that query string and set the page based on the result

screen_tes.mp4

@vercel
Copy link

vercel bot commented Feb 6, 2023 •
edited
Loading

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

Name Status Preview Comments Updated
dao ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 3, 2023 at 5:29PM (UTC)
pioneer-2 ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 3, 2023 at 5:29PM (UTC)
pioneer-2-storybook ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 3, 2023 at 5:29PM (UTC)

@vrrayz vrrayz requested review from thesan, traumschule and rafalpawlow and removed request for thesan and traumschule February 6, 2023 20:41
@vrrayz vrrayz mentioned this pull request Feb 6, 2023
Closed
1 task
@chrlschwb chrlschwb requested review from oleksanderkorn and removed request for thesan and rafalpawlow February 7, 2023 13:33
@chrlschwb chrlschwb added community-dev issue suitable for community-dev pipeline builders-wg-code-review labels Feb 7, 2023
traumschule
traumschule suggested changes Feb 7, 2023
View reviewed changes
Copy link
Collaborator

@traumschule traumschule left a comment

Choose a reason for hiding this comment

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

Tried to test with this thread: The page is empty and has no navigation. Please check.

packages/ui/src/forum/constant/pagination.ts Outdated Show resolved Hide resolved
packages/ui/src/forum/hooks/useForumCategoryThreadPage.ts Outdated Show resolved Hide resolved
packages/ui/src/common/components/Pagination/Pagination.tsx Show resolved Hide resolved
@vrrayz
Copy link
Contributor Author

vrrayz commented Feb 7, 2023

Tried to test with this thread: The page is empty and has no navigation. Please check.

Thats because the total amount of posts under this thread isn't enough to reach to a page 2 or 3. I could set it up to redirect to page 1 once the users try getting to anything greater than the pageCount. would that be okay?

@vrrayz vrrayz requested a review from traumschule February 7, 2023 19:46
@traumschule
Copy link
Collaborator

That's a good idea. Also pagination navigation can be hidden / disabled depending on number of posts and current page.

@vrrayz
Copy link
Contributor Author

vrrayz commented Feb 8, 2023

That's a good idea. Also pagination navigation can be hidden / disabled depending on number of posts and current page.

okay.. i'll get right to it

traumschule
traumschule approved these changes Feb 10, 2023
View reviewed changes
Copy link
Collaborator

@traumschule traumschule left a comment

Choose a reason for hiding this comment

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

LGTM

oleksanderkorn
oleksanderkorn approved these changes Feb 16, 2023
View reviewed changes
Copy link
Contributor

@oleksanderkorn oleksanderkorn left a comment

Choose a reason for hiding this comment

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

LGTM

@chrlschwb chrlschwb requested review from thesan and dmtrjsg February 24, 2023 09:26
thesan
thesan requested changes Feb 24, 2023
View reviewed changes
Copy link
Collaborator

@thesan thesan left a comment

Choose a reason for hiding this comment

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

Good job 👍 adding the page to the history is a good step forward.

There are still a bunch of issues with this back button but I think they can be taken care of in a follow up PR.

Just please add response to my comment 👇

packages/ui/src/forum/components/PostList/PostList.tsx Outdated Show resolved Hide resolved
@vrrayz vrrayz requested a review from thesan March 3, 2023 17:25
@thesan thesan added jsg-code-review waiting-for-checks PR is reviewed and approved, but is waiting for checks to complete labels Mar 3, 2023
@thesan thesan merged commit 0e35241 into Joystream:dev Mar 3, 2023
@thesan thesan removed the waiting-for-checks PR is reviewed and approved, but is waiting for checks to complete label Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thesan thesan thesan approved these changes

@oleksanderkorn oleksanderkorn oleksanderkorn approved these changes

@traumschule traumschule traumschule approved these changes

@chrlschwb chrlschwb Awaiting requested review from chrlschwb

@dmtrjsg dmtrjsg Awaiting requested review from dmtrjsg

Assignees

@vrrayz vrrayz

Labels
community-dev issue suitable for community-dev pipeline jsg-code-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vrrayz @traumschule @thesan @oleksanderkorn @chrlschwb