-
Notifications
You must be signed in to change notification settings - Fork 441
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
[stable17] Do not load Talk sidebar in public share page of folder shares #2340
[stable17] Do not load Talk sidebar in public share page of folder shares #2340
Conversation
lib/PublicShare/TemplateLoader.php
Outdated
*/ | ||
public static function loadTalkSidebarUi() { | ||
public static function loadTalkSidebarUi(?IShare $share) { | ||
if ($share && $share->getNodeType() !== FileInfo::TYPE_FILE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to switch to "hide by default".
This way we can help people to update their Nextcloud, as well as not allow to load the app on folders while the owner can not load the conversation in his file list?
if ($share && $share->getNodeType() !== FileInfo::TYPE_FILE) { | |
if (!$share || $share->getNodeType() !== FileInfo::TYPE_FILE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Talk sidebar was loaded on folders, but it was not shown, so that was not a problem. Anyway, if you want to help people to update their Nextcloud, fine by me.
258a6f1
to
e1bd518
Compare
|
The Talk sidebar is only shown for file shares, so there is no need to load it for folder shares. Moreover, this also prevents some of the hacks used to show the Talk sidebar to mess with the layout used for folders. Note that the "Files_Sharing::loadAdditionalScripts" event only includes the share since Nextcloud 17.0.1, so it is needed to handle the event without parameters to keep compatibility with Nextcloud 17.0.0 (in that case the Talk sidebar will not be loaded in any public share page until the server is updated to 17.0.1 or later). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
e1bd518
to
cf3eea4
Compare
Rebased due to conflicts with #2342. |
Well, we use talk everyday. Please list it as a bug in order to get it solved. It was not addressed in RC1 of 17 which is a bit annoying. |
Hi @FrankMaute It is not fixed in RC1, because Talk is not bundled with the server package. |
Sounds really great. Thanks for the support. |
Backport of #2339
Requires nextcloud/server#17597
Fixes #2282
Fixes #2293
Note that the
Files_Sharing::loadAdditionalScripts
event only includes the share since Nextcloud 17.0.1, so it is needed to handle the event without parameters to keep compatibility with Nextcloud 17.0.0 (in that case the Talk sidebar will still be loaded for folder shares).