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

fix(files): Adjust margin at the file list bottom #47467

Merged
merged 2 commits into from
Aug 26, 2024
Merged

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Aug 24, 2024

Summary

IIRC this was added when the drop notice was placed on the bottom, but the notice is now always set on the top. So this just causes weird whitespace on the files list.

Screenshots (scrolled fully down)

before after
Screenshot 2024-08-24 at 17-11-47 Files - Nextcloud Screenshot 2024-08-24 at 17-34-36 Files - Nextcloud
Screenshot 2024-08-24 at 17-12-38 Files - Nextcloud Screenshot 2024-08-24 at 17-35-02 Files - Nextcloud

Checklist

@susnux susnux added bug design Design, UI, UX, etc. 3. to review Waiting for reviews feature: files labels Aug 24, 2024
@susnux susnux added this to the Nextcloud 31 milestone Aug 24, 2024
@susnux susnux requested a review from skjnldsv as a code owner August 24, 2024 15:39
@susnux susnux changed the title fix(files): Remove huge margin on bottom fix(files): Remove huge margin at the file list bottom Aug 24, 2024
@skjnldsv
Copy link
Member

IIRC this was added when the drop notice was placed on the bottom, but the notice is now always set on the top. So this just causes weird whitespace on the files list.

This was also more visually appealing.
It gives a feeling of completion and you know you've reached the bottom.
I don't really mind, whatever we go for here, just adding some old input I remember hearing about :)

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Yes, as @skjnldsv mentioned, the bottom spacing is intentional, so that:

  • It is obvious you reached the end and it’s not loading anymore
  • The last items and summary are nicely readable in the center of the screen and not stuck at the bottom

(Some other apps like some text editors do this similarly.)

@susnux
Copy link
Contributor Author

susnux commented Aug 26, 2024

It is obvious you reached the end and it’s not loading anymore

I thought exactly the opposite, as scrolling into a blank page looks like something needs to load there 😅

But if this was a design decision, then maybe at least adjust the number to not be hard coded pixels but something variable like 25vh, so it works on all screens?

@artonge
Copy link
Contributor

artonge commented Aug 26, 2024

(Some other apps like some text editors do this similarly.)

But this is not a text editor. Cursed words incoming: "What's the competition doing?". I don't think this is expected from a file list, or any list in general, no?

@jancborchardt
Copy link
Member

But if this was a design decision, then maybe at least adjust the number to not be hard coded pixels but something variable like 25vh, so it works on all screens?

Sounds good @susnux! :)

@artonge it looks better, and some times inbetween the space was also missing and it resulted in it looking off. It’s just weird when the last files and the summary is stuck to the bottom of the screen.

@susnux
Copy link
Contributor Author

susnux commented Aug 26, 2024

I can understand both (personally I also tend to say a list that overflows one page should only be scroll able to the bottom), but also the design decision to have the margin seems reasonable, so I will adjust the PR to be relative to screen size.

(Because whats worse is e.g. VS code where you can scroll a whole page below the last line of code 🙈 )

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Thank you! :)

@jancborchardt jancborchardt changed the title fix(files): Remove huge margin at the file list bottom fix(files): Adjust margin at the file list bottom Aug 26, 2024
@susnux
Copy link
Contributor Author

susnux commented Aug 26, 2024

Done, looks like this now:

before after
Screen Shot 2024-08-26 at 16 29 46 Screen Shot 2024-08-26 at 16 28 53

Instead make it relative to screen size.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux enabled auto-merge August 26, 2024 14:33
@susnux susnux merged commit 6d31abd into master Aug 26, 2024
113 checks passed
@susnux susnux deleted the fix/virtual-files-list branch August 26, 2024 15:44
@susnux
Copy link
Contributor Author

susnux commented Aug 26, 2024

/backport to stable30

@susnux
Copy link
Contributor Author

susnux commented Aug 26, 2024

/backport to stable29

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.

4 participants