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

Change the batch size of folder content #2654

Merged
merged 2 commits into from
Sep 6, 2021
Merged

Change the batch size of folder content #2654

merged 2 commits into from
Sep 6, 2021

Conversation

iFlameing
Copy link
Member

@tisto screenshot

Screenshot 2021-09-03 at 4 05 54 PM

@tisto
Copy link
Member

tisto commented Sep 3, 2021

@plone/volto-team we would like to change the default batch size in the folder_contents to 50 and remove the 15 and 30 options. Are you folks ok with that? We'd like another review before merging.

@nzambello
Copy link
Member

@tisto I think changing the default can be ok, but why removing other options?

@tisto
Copy link
Member

tisto commented Sep 3, 2021

@nzambello batching options of 15 and 30 just do not make any sense IMHO. Why would you want to switch to 15/30 when you see all the items in 50? This is just a leftover from ancient times of the web, they do not make any sense today.

@nzambello
Copy link
Member

nzambello commented Sep 3, 2021

@tisto you're right.
I have not any blocker here, I think it's fine.
If anybody for any reason would need that, we can think of an option for this, so the pageSizes are configurable in the project.

@tisto
Copy link
Member

tisto commented Sep 3, 2021

@nzambello in the long run I'd like to get rid of this entirely. I see the reason for an "all" button but batching does not make much sense. Maybe we can lazy load content or just add a "more" button at the end which is more user-friendly IMHO.

@avoinea
Copy link
Member

avoinea commented Sep 3, 2021

😄 Good luck with that when you're folder contains 5.000-10.000 children.

@tisto
Copy link
Member

tisto commented Sep 3, 2021

@avoinea with 10k objects, it does not make any difference. The default batch size is 50 that always works. If you have 10k objects and click on "all" you are screwed, no matter if we stick with the current approach or improve it.

@avoinea
Copy link
Member

avoinea commented Sep 6, 2021

@tisto True, for the All button. I was referring to this statement but batching does not make much sense.

But indeed, maybe with a smart filter and a sorting functionality, this may be dropped.

The use case I have in mind is when you have 10k items and you want to go to the last page and remove old items. Or, I don't know see all items published 5 years ago.

@tisto
Copy link
Member

tisto commented Sep 6, 2021

@avoinea I guess we fully agree here. I'd like to replace batching if possible. Though, we have to take all the use cases into account and make sure they will still work when we change things. The current PR is a good compromise. We can move in the right direction and start a discussion about where we might want to go. :)

@tisto tisto merged commit 6343e0c into master Sep 6, 2021
@tisto tisto deleted the batch-size branch September 6, 2021 09:07
sneridagh added a commit that referenced this pull request Sep 7, 2021
* master: (167 commits)
  Back to development
  Release 13.15.0
  Prepare for release
  feat: add placeholder to wysiwyg widget (#2653)
  chore(i18n): update ita translations (#2652)
  Show loading indicator in listing view when content is loading (#2644)
  Validate touched-only `required` fields in Forms (#2642)
  Get SchemaWidget field factories from backend (#2651)
  Change the batch size of folder content (#2654)
  Show item title and item type when hovering over item title and item type icon in folder content view (#2649)
  Back to development
  Release 13.14.0
  Prepare for release
  cypress: user/groups controlpanel tests (#2650)
  Reimplement the architecture of user/groups controlpanel (#2064)
  Back to development
  Release 13.13.0
  Prepare for release
  Showing version in History view (#2634)
  Fix SearchWidget required pathname (#2646)
  ...
sneridagh added a commit that referenced this pull request Sep 10, 2021
* master: (34 commits)
  Back to development
  Release 14.0.0-alpha.0
  Prepare for release
  Remove compatibility with older configuration way based on imports (#2516)
  Locking support (#2594)
  Back to development
  Release 13.15.0
  Prepare for release
  feat: add placeholder to wysiwyg widget (#2653)
  chore(i18n): update ita translations (#2652)
  Show loading indicator in listing view when content is loading (#2644)
  Validate touched-only `required` fields in Forms (#2642)
  Get SchemaWidget field factories from backend (#2651)
  Change the batch size of folder content (#2654)
  Show item title and item type when hovering over item title and item type icon in folder content view (#2649)
  Back to development
  Release 13.14.0
  Prepare for release
  cypress: user/groups controlpanel tests (#2650)
  Reimplement the architecture of user/groups controlpanel (#2064)
  ...
sneridagh added a commit that referenced this pull request Sep 13, 2021
* master: (79 commits)
  Back to development
  Release 14.0.0-alpha.0
  Prepare for release
  Remove compatibility with older configuration way based on imports (#2516)
  Locking support (#2594)
  Back to development
  Release 13.15.0
  Prepare for release
  feat: add placeholder to wysiwyg widget (#2653)
  chore(i18n): update ita translations (#2652)
  Show loading indicator in listing view when content is loading (#2644)
  Validate touched-only `required` fields in Forms (#2642)
  Get SchemaWidget field factories from backend (#2651)
  Change the batch size of folder content (#2654)
  Show item title and item type when hovering over item title and item type icon in folder content view (#2649)
  Back to development
  Release 13.14.0
  Prepare for release
  cypress: user/groups controlpanel tests (#2650)
  Reimplement the architecture of user/groups controlpanel (#2064)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants