Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

feat: add total to paginatedPage and clear the extra queries #2340

Conversation

Sboonny
Copy link
Member

@Sboonny Sboonny commented Feb 5, 2023

  • I have read Chapter's contributing guidelines.
  • My pull request has a descriptive title (not a vague title like Update README.md).
  • My pull request targets the main branch of Chapter.

I am trying to observe the data, and hide the data based on if there is more data or not, and when it's complete and there is no more data, we can hide the buttons, instead of running the function again to find out if there is data left.


Apparently if you ruin the git history, and decide to discard the changes, for new one good commit. GitHub close the open PR and you have to open new one. 🤔 To be honest, I am missing something with how to do it cleanly with Git so no need to focus on that PR anymore, because I can't update commits to it #2294

@Sboonny Sboonny requested a review from a team February 5, 2023 08:18
@Sboonny Sboonny marked this pull request as draft February 5, 2023 08:18
@ghost
Copy link

ghost commented Feb 5, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

Sboonny and others added 3 commits February 5, 2023 10:09
Co-authored-by: Aashutosh Poudel <aashutoshpoudyal@gmail.com>
Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
@Sboonny Sboonny marked this pull request as ready for review February 5, 2023 10:54
@Sboonny Sboonny added UI/UX This issue deals with UI/UX API labels Feb 5, 2023
@Sboonny Sboonny force-pushed the feat/hide-hasmore-button-when-there-is-no-events branch from be906d0 to 7c5df3f Compare February 21, 2023 17:16
@Sboonny
Copy link
Member Author

Sboonny commented Feb 22, 2023

Will be affect by #2224, so I am blocking it for now

@Sboonny Sboonny marked this pull request as draft February 27, 2023 16:46
@Sboonny Sboonny force-pushed the feat/hide-hasmore-button-when-there-is-no-events branch from e0af6fb to 8015bba Compare February 27, 2023 16:51
Co-authored-by: Krzysztof G. <60067306+gikf@users.noreply.github.com>
@Sboonny Sboonny force-pushed the feat/hide-hasmore-button-when-there-is-no-events branch from 8015bba to e3e60c2 Compare February 27, 2023 16:58
@Sboonny Sboonny marked this pull request as ready for review February 28, 2023 06:58
@gikf
Copy link
Member

gikf commented Feb 28, 2023

@Sboonny would you mind if I push couple commits directly? I've made further changes to make Pagination more generic. It'd be more convenient to share it like that, instead of making dozen suggestions to make github happy.

@Sboonny
Copy link
Member Author

Sboonny commented Feb 28, 2023

Nope, feel free to 👍, remove it from draft when you are done

@Sboonny Sboonny marked this pull request as draft February 28, 2023 13:42
@gikf gikf marked this pull request as ready for review February 28, 2023 21:09
@gikf
Copy link
Member

gikf commented Mar 1, 2023

@ojeytonwilliams If you could take a look and review this in spare time, with fresh pair of eyes, that'd be great.

Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

Hey @gikf @Sboonny, I like the look and feel. I think there's a few minor tweaks we can make, though:

client/src/modules/util/pagination.tsx Outdated Show resolved Hide resolved
client/src/modules/home/index.tsx Outdated Show resolved Hide resolved
client/src/modules/events/pages/eventsPage.tsx Outdated Show resolved Hide resolved
server/src/controllers/Events/resolver.ts Outdated Show resolved Hide resolved
server/src/controllers/Events/resolver.ts Outdated Show resolved Hide resolved
server/src/controllers/Events/resolver.ts Outdated Show resolved Hide resolved
@gikf
Copy link
Member

gikf commented Mar 1, 2023

Thanks @ojeytonwilliams. I was very happy with my making Pagination as abstract 😆, you are right that's going a bit too far and it can be simpler.

@Sboonny let me know if you'd want me to make suggested changes to the Pagination myself.

@Sboonny
Copy link
Member Author

Sboonny commented Mar 1, 2023

Sorry for the delay, @gikf. I don't understand the requested changes, so part of me want to change it and find out more.

But I know how obnoxious I can be, when I don't understand something. Give me tomorrow, I will try to implement it, if I can't, I will ping you.

@Sboonny Sboonny marked this pull request as draft March 2, 2023 10:39
Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
@Sboonny Sboonny marked this pull request as ready for review March 2, 2023 12:47
@Sboonny Sboonny force-pushed the feat/hide-hasmore-button-when-there-is-no-events branch from e607cf9 to b4c9497 Compare March 2, 2023 12:55
Co-authored-by: Oliver Eyton-Williams <ojeytonwilliams@gmail.com>
Co-authored-by: gikf <60067306+gikf@users.noreply.github.com>
@Sboonny Sboonny force-pushed the feat/hide-hasmore-button-when-there-is-no-events branch from b4c9497 to d93ae7f Compare March 2, 2023 12:58
Copy link
Contributor

@ojeytonwilliams ojeytonwilliams left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Since I pushed a bit of code, could you sign off on this @gikf ?

Also, thanks to you both for your hard work on this. I didn't expect cleaning up the Click for more button to be anywhere near this involved, but what you came up with is a big improvement.

@gikf gikf merged commit eb87464 into freeCodeCamp:main Mar 2, 2023
@Sboonny Sboonny deleted the feat/hide-hasmore-button-when-there-is-no-events branch March 2, 2023 20:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API status: ready for review UI/UX This issue deals with UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants