Skip to content

Only use prev and next buttons for pagination on user dashboard #33981

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
Merged
merged 7 commits into from
Mar 23, 2025
Merged

Only use prev and next buttons for pagination on user dashboard #33981

merged 7 commits into from
Mar 23, 2025

Conversation

lunny
Copy link
Member

@lunny lunny commented Mar 23, 2025 •
edited
Loading

The pagination on the user dashboard sounds unnecessary, this will change it to a prev/next buttons. For instances with around 10 million records in the action table, this option affects how the user dashboard is loaded on first visit.

Before:

image

After:

image

@lunny lunny added the performance/speed performance issues with slow downs label Mar 23, 2025
@lunny lunny added this to the 1.24.0 milestone Mar 23, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 23, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files docs-update-needed The document needs to be updated synchronously labels Mar 23, 2025
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 23, 2025
wxiaoguang
wxiaoguang previously requested changes Mar 23, 2025
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Why a new unnecessary config option and why keep copying-pasting the code , and why math.MaxInt32

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 23, 2025
@wxiaoguang
Copy link
Contributor

It should make a "Paginator" like this: Pages=nil, HasPrev=true, HasNext=true

@lunny
Copy link
Member Author

lunny commented Mar 23, 2025

It should make a "Paginator" like this: Pages=nil, HasPrev=true, HasNext=true

Did you mean reuse the original template? Then we still need to handle IsFirst and IsLast. The IsLast is not easy to handle.

wxiaoguang
wxiaoguang reviewed Mar 23, 2025
View reviewed changes
@pull-request-size pull-request-size bot added size/S and removed size/M labels Mar 23, 2025
@github-actions github-actions bot removed modifies/templates This PR modifies the template files docs-update-needed The document needs to be updated synchronously labels Mar 23, 2025
@pull-request-size pull-request-size bot added size/M and removed size/S labels Mar 23, 2025
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Mar 23, 2025
@wxiaoguang wxiaoguang force-pushed the lunny/allow_no_pagination_user_dashboard branch from e2f13c0 to 19d11c3 Compare March 23, 2025 08:07
@wxiaoguang wxiaoguang force-pushed the lunny/allow_no_pagination_user_dashboard branch from 19d11c3 to 69aa478 Compare March 23, 2025 08:10
@wxiaoguang wxiaoguang dismissed their stale review March 23, 2025 08:12

dismiss

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Mar 23, 2025
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 23, 2025
@lunny
Copy link
Member Author

lunny commented Mar 23, 2025

Is it necessary to display the current page?

image

@wxiaoguang
Copy link
Contributor

Is it necessary to display the current page?

I believe yes. Otherwise, how could a user know which page they are viewing?

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 23, 2025
@lunny lunny enabled auto-merge (squash) March 23, 2025 18:54
@wxiaoguang wxiaoguang disabled auto-merge March 23, 2025 19:01
@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 23, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 23, 2025 •
edited
Loading

Still need to fix some edge cases:

  • the "empty activity" prompt
  • the "last page"

Ideally we should write some tests for the "user dashboard page" to check various cases.

image

image

image

@wxiaoguang wxiaoguang force-pushed the lunny/allow_no_pagination_user_dashboard branch from f10dd0b to 9271280 Compare March 23, 2025 19:21
@wxiaoguang wxiaoguang force-pushed the lunny/allow_no_pagination_user_dashboard branch from 9271280 to 1f790ec Compare March 23, 2025 19:27
@wxiaoguang wxiaoguang enabled auto-merge (squash) March 23, 2025 19:29
@wxiaoguang wxiaoguang changed the title Don't display pagination on user dashboard by default but with prev and next buttons Only use prev and next buttons for pagination on user dashboard Mar 23, 2025
@wxiaoguang wxiaoguang disabled auto-merge March 23, 2025 19:29
@wxiaoguang wxiaoguang enabled auto-merge (squash) March 23, 2025 19:30
@wxiaoguang wxiaoguang merged commit 30ee082 into go-gitea:main Mar 23, 2025
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 23, 2025
@lunny lunny deleted the lunny/allow_no_pagination_user_dashboard branch March 23, 2025 21:46
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 24, 2025
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
fde02d1 
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Only use prev and next buttons for pagination on user dashboard (go-gitea#33981)
  update jwt and redis packages (go-gitea#33984)
  [skip ci] Updated translations via Crowdin
  Improve oauth2 error handling (go-gitea#33969)
  [skip ci] Updated translations via Crowdin
  Cover `go.mod` and `go.sum` in `.editorconfig` (go-gitea#33960)
  Drop timeout for requests made to the internal hook api (go-gitea#33947)
  Fix file name could not be searched if the file was not a text file when using the Bleve indexer (go-gitea#33959)
  Fix oauth2 auth and UI (go-gitea#33961)
  Allow filtering issues by any assignee (go-gitea#33343)
  Optimize total count of feed when loading activities in user dashboard. (go-gitea#33841)
  Extract code to their own functions for push update (go-gitea#33944)
  Optimize heatmap query (go-gitea#33853)
@lunny lunny mentioned this pull request Apr 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@appleboy appleboy appleboy approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files performance/speed performance issues with slow downs
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.
4 participants
@lunny @wxiaoguang @appleboy @GiteaBot