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 sidebar not automatically hidden in Files app #35052

Merged

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Nov 9, 2022

Fixes #33848

This might be happening since #15719 (but I have not verified it)

Although the Files app creates the legacy sidebar (details view) it is then replaced with the newer Vue app sidebar. Due to this .detailsView no longer finds an element and therefore nothing was hidden when hideAppSidebar($('.detailsView')) was called (for example, when changing to another section).

However, OC.Apps.hideAppSidebar() does not properly work either with the Vue sidebar used in the Files app (once hidden the sidebar is not shown again). For simplicity, and to avoid any possible side effect in other apps from changing OC.Apps.hideAppSidebar, now OC.Files.Sidebar.close() is used instead.

How to test

  • Open the Files app
  • Show the sidebar for any file
  • Change to a different section (Recent, Favorites...)

Result with this pull request

The sidebar is hidden

Result without this pull request

The sidebar is not hidden

@danxuliu danxuliu added this to the Nextcloud 26 milestone Nov 9, 2022
@danxuliu danxuliu requested review from skjnldsv, a team, Pytal and szaimen and removed request for a team November 9, 2022 12:53
@danxuliu
Copy link
Member Author

danxuliu commented Nov 9, 2022

/backport to stable25

@danxuliu
Copy link
Member Author

danxuliu commented Nov 9, 2022

/backport to stable24

@danxuliu
Copy link
Member Author

danxuliu commented Nov 9, 2022

/backport to stable23

@nickvergessen
Copy link
Member

3rdparty change should not be here?

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Blocking due to unintended 3rdparty bump

Although the Files app creates the legacy sidebar (details view) it is
then replaced with the newer Vue app sidebar. Due to this ".detailsView"
no longer finds an element and therefore nothing was hidden when
"hideAppSidebar($('.detailsView'))" was called (for example, when
changing to another section).

However, "OC.Apps.hideAppSidebar()" does not properly work either with
the Vue sidebar used in the Files app (once hidden the sidebar is not
shown again). For simplicity, and to avoid any possible side effect in
other apps from changing "OC.Apps.hideAppSidebar", now
"OC.Files.Sidebar.close()" is used instead.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the fix-sidebar-not-hidden-when-changing-section-in-files-app branch from bb799d3 to a8fea27 Compare November 9, 2022 16:46
@danxuliu
Copy link
Member Author

danxuliu commented Nov 9, 2022

3rdparty change should not be here?

Ugh 🤦 Sorry, fixed now.

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Seems like it fixes #33848 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

switching to any of the other view does not close the sidebar
5 participants