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

feat(sharing): Allow to set default view mode for public shares #50739

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 8, 2025

Summary

Allow to set the default view mode for public shares.
User shares allow the user to define it themself and saves it on their user config, but for public shares this is not available.
So let the sharer allow to define the default view mode.

Screen shots

New config option:

Screenshot 2025-02-10 at 13-17-51 Files - Nextcloud

New behavior:

Bildschirmaufnahme_20250210_133103.webm

Checklist

@susnux susnux added this to the Nextcloud 32 milestone Feb 8, 2025
@susnux
Copy link
Contributor Author

susnux commented Feb 8, 2025

/backport to stable31

nfebe

This comment was marked as resolved.

@susnux

This comment was marked as resolved.

@susnux susnux requested review from artonge and nfebe February 10, 2025 12:24
Copy link
Contributor

@nfebe nfebe left a comment

Choose a reason for hiding this comment

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

LGTM code and function.

However it feels off to have view controls in the share settings (I am aware this was there before) but maybe the toggle/checkbox can be moved out and displayed on the actual file list controls.

cc: @marcoambrosini

Copy link
Contributor

@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.

Similar concern as Fon regarding the cost/benefice of this feature. Where the cost is having yet another config option.

@marcoambrosini
Copy link
Member

I agree with the concern raised @susnux
Anything against just defaulting with grid-view for share links?

@susnux
Copy link
Contributor Author

susnux commented Feb 10, 2025

Anything against just defaulting with grid-view for share links?

Yes it only makes sense for images / videos. I would say thats not the majority of Nextcloud use cases.
If you share files it makes no sense to see it in list view.

Where the cost is having yet another config option.

Well its just a checkbox, nothing new in the backend.


I think we have both use cases of sharing images and sharing documents / files.
From my experience (but that might be biased) in most cases it is about sharing documents where list view makes sense, but of course some people might want to share images where the grid view makes sense.

Haven an option here allows users to decide which one is the best based on their use case instead of some use case we assume.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the feat/share-grid-view branch from 86fda30 to d8a365c Compare February 10, 2025 17:59
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

🆗

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

actually blocking since more discussion is happening

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

clarified, green light from my side

@susnux susnux merged commit 5f423df into master Feb 11, 2025
119 of 120 checks passed
@susnux susnux deleted the feat/share-grid-view branch February 11, 2025 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to default to grid view when sharing link
4 participants