plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
448 stars 610 forks source link

File upload modal needs `overflow: auto` and `max-height` #5677

Open Ayush-2301 opened 8 months ago

Ayush-2301 commented 8 months ago

Is your feature request related to a problem? Please describe.

When uploading too many files to a folder content of a page, the modal height increases, affecting the user experience. The 'Save' or 'Cancel' button may also be pushed down.

To Reproduce

Steps to reproduce the behavior:

As admin Go to demo.plone.org Navigate to folder_contents. Drag and drop or add multiple files.

Describe the solution you'd like

A pagination component is need to be added in the file upload modal

Additional context

https://github.com/plone/volto/assets/114101923/2b9946b6-709a-4ac8-b964-4e628e45f57d

Ayush-2301 commented 8 months ago

working on it

stevepiercy commented 8 months ago

working on it

@Ayush-2301 clearly you did not read and follow item 2 in https://6.docs.plone.org/contributing/first-time.html#work-with-github-issues.

ichim-david commented 8 months ago

working on it

@Ayush-2301 clearly you did not read and follow item 2 in https://6.docs.plone.org/contributing/first-time.html#work-with-github-issues.

@stevepiercy I think in this case his comment was correct to signal to other individuals that he added the issue and is working on it so that others don't try to race to the board on who opens a pull request for this issue :).

@Ayush-2301 now that you have been added to the plone repository, do you have permission to assign your name to the asignees field? assign-to-issue

stevepiercy commented 8 months ago

@ichim-david @Ayush-2301 sorry, I meant to say item 3, not 2, in Work with GitHub issues.

Discussing what the correct implementation would be, including the possibility of reusing existing components, instead of reinventing the wheel, or worse, adding a new dependency, could save time for both contributors and reviewers.

As far as racing in, "Fools rush in where angels fear to tread".

stevepiercy commented 8 months ago

@ichim-david the Contributors Team (read role) lacks a lot of permissions that the Developers Team (write role) have, and assignment of issues is one of those. Here's the complete list of permissions for all roles. https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization#permissions-for-each-role. We wish GitHub would grant customization of these roles for open source organizations, but they reserve it for paid Enterprise accounts only.

ichim-david commented 8 months ago

@Ayush-2301 before rushing in on working on this issue, as @stevepiercy mentioned I think you should wait for an extra comment on the validity of this enhancement as I know there were some pull requests for folder_contents with enhancements plus other issues so I suggest you wait a bit longer before you start working on this.

Ayush-2301 commented 8 months ago

I haven't started the work yet , I've only looked into the codebase .The volto codebase already have pagination logic and its separate component. The component Pagination is present in ./packages/volto/src/components/theme/Pagination/Pagination.jsx and is used in Contents component ./packages/volto/components/manage/Contents/Contents.jsx . I was planning to use the same Pagination component in the ContentsUploadModal component.

@ichim-david , I will wait for the approval before proceeding with the work. Apologies for any inconvenience.

ichim-david commented 7 months ago

@plone/volto-team what is your opinion on this issue?

Personally, if the concern is about not seeing the upload or cancel buttons I would add an overflow auto and a max-height as seen in this screenshot overflow-upload-dialog

As such I don't think it's worth complicating the upload popup with a pagination but maybe I'm wrong and I welcome your opinion.

stevepiercy commented 7 months ago

I dislike pagination as it makes it difficult to find that one file in the list with a CMD-F. I prefer @ichim-david's simple, elegant, fast, and probably smaller file size suggestion.

Ayush-2301 commented 7 months ago

@ichim-david I like your approach it is much better . I don't know why i didn't thought about it . I believe this approach solve this issue as well as issue #5620