Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Jan 30, 2025

  • Resolves: #

Summary

  1. We did not consider the files filter height for the calculation
  2. Initialize height variables before using them for scrolling - and wait for tbody styles to be updated.
  3. Fixed condition to skip scrolling if there are less rows available then the current viewport can render.
  4. Correctly clamp index we can scroll to when we reach the end of the scrollable area.

There is a slight change in design:
We scroll in such a way now that users see half of the previous row. The previous indented behavior was to see one and a half rows.
Reasons:

  1. In grid mode the selected row is hard to spot on small screens
  2. (IMHO) it is generally better to directly show the user the row directly (while keeping half a row on top to clarify the list is scrolled)

⬆️ we can discuss this change and drop it if preferred.

Screenshots

This is the scrolling position when having 11 files (0 to 10) and scrolling to index N:

Selected file (index) 🖼️
0 Bildschirmfoto am 2025-01-30 um 16 57 30
1 Bildschirmfoto am 2025-01-30 um 16 57 37
2 Bildschirmfoto am 2025-01-30 um 16 57 42
3 Bildschirmfoto am 2025-01-30 um 16 57 47
4 Bildschirmfoto am 2025-01-30 um 16 57 52
5 Bildschirmfoto am 2025-01-30 um 16 57 57
6 Bildschirmfoto am 2025-01-30 um 16 58 02
7 Bildschirmfoto am 2025-01-31 um 00 20 07
8,9,10 (same for all) Bildschirmfoto am 2025-01-30 um 16 58 07

Checklist

@susnux susnux added bug design Design, UI, UX, etc. 3. to review Waiting for reviews feature: files labels Jan 30, 2025
@susnux susnux added this to the Nextcloud 32 milestone Jan 30, 2025
@susnux susnux marked this pull request as ready for review January 31, 2025 06:44
@susnux susnux requested a review from skjnldsv as a code owner January 31, 2025 06:44
provokateurin

This comment was marked as resolved.

@susnux

This comment was marked as resolved.

@provokateurin

This comment was marked as resolved.

@susnux susnux force-pushed the fix/scrolling-file-list branch from 23e5ce5 to e4bd5f8 Compare January 31, 2025 12:49
@susnux susnux force-pushed the fix/scrolling-file-list branch from bf2cbbb to 423a91d Compare February 3, 2025 20:51
@susnux
Copy link
Contributor Author

susnux commented Feb 3, 2025

/compile

@skjnldsv
Copy link
Member

skjnldsv commented Feb 4, 2025

We scroll in such a way now that users see half of the previous row. The previous indented behavior was to see one and a half rows.

Good for me!! 👍

@skjnldsv
Copy link
Member

skjnldsv commented Feb 4, 2025

On your screenshots, why is the row not active ?
If the fileid is selected, it should be.

Is this a screenshot glitch ? Or is it broken ? (because it was working not too long ago 🙈 )

@susnux
Copy link
Contributor Author

susnux commented Feb 4, 2025

Is this a screenshot glitch ? Or is it broken ?

Screenshot glitch 🙈
Confirmed it is still working ✅

@susnux susnux force-pushed the fix/scrolling-file-list branch 3 times, most recently from 6bebeca to 7fe7c61 Compare February 5, 2025 11:55
@susnux susnux force-pushed the fix/scrolling-file-list branch from 7fe7c61 to 9ef48d7 Compare February 5, 2025 17:40
@susnux susnux merged commit c5e24b5 into master Feb 5, 2025
120 checks passed
@susnux susnux deleted the fix/scrolling-file-list branch February 5, 2025 21:14
@susnux
Copy link
Contributor Author

susnux commented Feb 5, 2025

/backport to stable31

@susnux
Copy link
Contributor Author

susnux commented Feb 5, 2025

/backport to stable30

@susnux
Copy link
Contributor Author

susnux commented Feb 5, 2025

/backport to stable31

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 design Design, UI, UX, etc. feature: files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants