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_sharing): Hide download should not prevent download #50897

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nfebe
Copy link
Contributor

@nfebe nfebe commented Feb 19, 2025

Hide download is meant to hide the download button from the UI not enforce the download:false attribute.

This commit also modifies the advanced options to only show "Allow download and sync" on public shares.

Resolves : #48954

Hide download is meant to hide the download button from the UI not enforce
the `download:false` attribute.

This commit also modifies the advanced options to only show "Allow download and sync" on
public shares.

Signed-off-by: nfebe <[email protected]>
@@ -169,7 +169,7 @@
@update:checked="queueUpdate('hideDownload')">
{{ t('files_sharing', 'Hide download') }}
</NcCheckboxRadioSwitch>
<NcCheckboxRadioSwitch v-else
<NcCheckboxRadioSwitch v-if="isPublicShare"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is internal share not public share

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need a way to users to undo, the effects of the bug introduced by merging hideDownload and download permissions.

Looking at the backend, it is the same control, so I am not sure why there distinction is there and it is needed internally however maybe we can leave it for both?

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Note that apps in the public page used the hideDownload element to know if they should hide the download or not. That element was also removed when the public page was moved to Vue, so the value should be added back somehow. For example, with $this->initialState->provideInitialState('hideDownload', $share->getHideDownload()); in DefaultPublicShareTemplateProvider.php similarly to what was done with the sharingToken value during the migration to Vue. Otherwise even if the download is hidden the apps in the public page would not know it.

Additionally it would be nice if something like isDownloadHidden() or something like that was added to @nextcloud/sharing. However this could be done at a later point, it would not be a must to restore the previous behaviour.

Edit: oh, and the Download button should also be hidden from the public share page :-) That was done in the past but lost with the Vue migration

@susnux
Copy link
Contributor

susnux commented Feb 20, 2025

@danxuliu this needs product management decision on #46256 so we know what to do.
Already pinged @sorbaugh on it.

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.

"Hide download" in public shares is no longer "hide download" but "prevent download" in Nextcloud 31
3 participants