Skip to content

Conversation

@starypatyk
Copy link
Contributor

When a folder contains over ~100 files or sub-folders (exact number seems to depend on the browser windows size), only a first batch of files/sub-folders is displayed. Empty space is shown when scrolling further down. This has been described in #1540 and likely also in #1490.

This seems to be the same issue as the one described in the vue-virtual-grid repository: Mikescops/vue-virtual-grid#25

The fix provided there (Mikescops/vue-virtual-grid#26) introduces a new property: scrollElement that should be set to an appropriate element on the page - i.e. the scrolling container.

In this PR I propose a fix implemented similarly to what is used e.g. in FaceContent.vue:

:container-element="appContent"
appContent: document.getElementById('app-content-vue'),

Signed-off-by: Dariusz Olszewski <[email protected]>
@starypatyk starypatyk added bug Something isn't working 3. to review Waiting for reviews labels Mar 9, 2023
@szaimen szaimen requested review from artonge and skjnldsv March 9, 2023 22:38
@szaimen szaimen added this to the Nextcloud 27 milestone Mar 9, 2023
Copy link
Collaborator

@artonge artonge left a comment

Choose a reason for hiding this comment

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

If this fix the issue in your testing, then it looks fine :)

@artonge artonge enabled auto-merge March 13, 2023 09:09
@artonge artonge merged commit 7d49671 into master Mar 13, 2023
@artonge artonge deleted the fix/1540_scrolling_large_folders branch March 13, 2023 09:17
@starypatyk
Copy link
Contributor Author

Hi @artonge,

Great to see the fix merged into master. 👍

Would you consider backporting it to stable26 and stable25?

The change applies cleanly to both these branches (except compiled JS of course). Pre-requisites are also there - i.e. the required element on the page and appropriate version of the vue-virtual-grid library.

I suspect I could invoke the nextcloud-bot myself, but I am not sure if this is an accepted practice for external contributors.

@skjnldsv
Copy link
Member

/backport to stable26

@skjnldsv
Copy link
Member

/backport to stable25

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Mar 19, 2023
@backportbot-nextcloud backportbot-nextcloud bot removed the backport-request Pending backport by the backport-bot label Mar 19, 2023
@artonge
Copy link
Collaborator

artonge commented Apr 18, 2023

I suspect I could invoke the nextcloud-bot myself, but I am not sure if this is an accepted practice for external contributors.

Stumbling on that, and it looks like we did not answer you. Feel free to use the backport bot next time if you feel like it is needed. In any case, the generated PRs will need to go through another round of approval ;).

@starypatyk
Copy link
Contributor Author

Thanks @artonge! As another PR is in progress, I will probably use it soon. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants