-
Notifications
You must be signed in to change notification settings - Fork 62
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
show all bounty on homepage #920
Conversation
@ecurrencyhodler you can check the video |
LGTM! Let's do a code review. We will need to test again in staging before we pay out. Thanks marvel. |
@@ -777,7 +777,7 @@ export class MainStore { | |||
// if we don't pass the params, we should use previous params for invalidate query | |||
const query2 = this.appendQueryParams( | |||
'gobounties/all', | |||
queryLimit, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a simpler change would to be to just increase this queryLimit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i thought of doing that. but I assumed there was a reason that it was made 100 initially. So i went with the route of if a query limit is passed that should be used and if not it default to the 100 @kevkevinpal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevkevinpal still waiting for a review
@Marvel-Ib your changes will break the bounty modal, fix this. |
@Marvel-Ib it looks like raph tested this by going to an old bounty 466. When he did, the website became unresponsive. That is a breaking change. Do you think you can address it? |
@Marvel-Ib So right now the code is filtering through an array. The load more button calls the backend but the filter only searches through an array. There will also need to be refactoring due to new endpoints we have. We should just call the "get bounty" endpoint. If you have any other questions, ping @elraphty We can also increase the bounty for this issue. |
Describe your changes
Enable the load more button on the homepage to show all bounties
Screen.Recording.2023-11-10.at.04.42.59.mov
Issue ticket number and link
#908
Type of change
Please delete options that are not relevant.
Checklist before requesting a review