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.

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

Add new settings for page number control #23428

Open
yp05327 opened this issue Mar 12, 2023 · 7 comments
Open

Add new settings for page number control #23428

yp05327 opened this issue Mar 12, 2023 · 7 comments
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@yp05327
Copy link
Contributor

yp05327 commented Mar 12, 2023

Feature Description

Reported by @hiifong

In #22937, we added pagination for user dashboard, but it broke the definition of FeedPagingNum in the document:
image
We need to add new settings for the page number control to avoid too many pages in user dashboard, as if there are too many records, it will be very slow to load the page.

Screenshots

No response

@yp05327 yp05327 added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Mar 12, 2023
@brechtvl
Copy link
Contributor

brechtvl commented Mar 12, 2023

It's not clear to me how FEED_PAGING_NUM is broken, as far as I can tell that still correctly controls the number of activities on each page?

I can see that counting the total number of activities for the purpose of the "Last" button could be slow. Two potential solutions:

  • Just hide the "Last" button for the activity feed, it's not important to be able to navigate to the last page for this case.
  • Add a mechanism like ?page=last which counts the total activities only after clicking "Last", so it doesn't affect typical dashboard page views.

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 12, 2023

In source code, it is defined as the number of feeds on each page, but in the document on each page is not mentioned.
So user may take it for the total number of all items.

ps:
At first, @hiifong reported the last page button is too slow.
Then @lunny said we have settings to control the total display count.
Then someone said maybe FeedPagingNum can control it , as the document said we can control the number of items that are displayed in home feed.
But it didn't work as FeedPagingNum should have a default vaule 20.
Then I found that in #22937, FeedPagingNum which can control the toltal display count in old versions, has been changed to control the number of items on each page.

@brechtvl
Copy link
Contributor

brechtvl commented Mar 12, 2023

Ah, to me it makes sense to just update the description to say "on one page" then since that's what the other *_PAGING_NUM settings do and still effectively the same as before for the first page.

Personally I think hiding the "Last" button would be preferable to capping the number of activities, since "Last" would be quite arbitrary then.

@lunny
Copy link
Member

lunny commented Mar 12, 2023

Maybe we should only display Prev and Next buttons, because it's meaningless to go to some page.

@hiifong
Copy link
Contributor

hiifong commented Mar 13, 2023

In source code, it is defined as the number of feeds on each page, but in the document on each page is not mentioned. So user may take it for the total number of all items.

ps: At first, @hiifong reported the last page button is too slow. Then @lunny said we have settings to control the total display count. Then someone said maybe FeedPagingNum can control it , as the document said we can control the number of items that are displayed in home feed. But it didn't work as FeedPagingNum should have a default vaule 20. Then I found that in #22937, FeedPagingNum which can control the toltal display count in old versions, has been changed to control the number of items on each page.

home page loading time screenshot
TQ%KT}S217J@R)SHPCUNB(F

@Zettat123
Copy link
Contributor

@hiifong Could you please add the index in #21611 (comment) and test if the index could improve the page loading time?

@yp05327
Copy link
Contributor Author

yp05327 commented Mar 14, 2023

Ah, to me it makes sense to just update the description to say "on one page" then since that's what the other *_PAGING_NUM settings do and still effectively the same as before for the first page.

As we have no version control for the old document, maybe we need to wait until the new document being finished to change the description?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

5 participants