Always replace the app sidebar view when switching from a different file list #10509
Conversation
apps/files/js/detailsview.js
Outdated
| } else { | ||
| $('#app-sidebar').replaceWith(this.$el) | ||
| } | ||
| this.$el.insertAfter($('#app-content')); |
There was a problem hiding this comment.
Yes, that will lead to multiple divs with the same id of app-content, but I'd still keep it like this as a temporary fix, as it was like that before as well.
There was a problem hiding this comment.
Okay, I fixed this before. The multiple content are not acceptable according to the acceptance tests, we removed all the old tests for this reason. I really think we should not do that.
Why the replacement of the sidebar with the new element does not work?
There was a problem hiding this comment.
The replacement is only done once when loading the list for the first time, when you try to show the details view for a second time, after a different view has been used, it will not call render and therefore not replace it again. But maybe that is a good idea to just make sure it gets replaced on each opening of the details sidebar. I'll have a look.
There was a problem hiding this comment.
What about replacing ids or stuff like that?
#app-sidebar-favorites, when selected -> #app-sidebar
There was a problem hiding this comment.
It was actually even easier: 16f6c07 I now just run the render function on every details sidebar change and just replace it if the element is from a different filelist.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Julius Härtl <[email protected]>
…pdated Signed-off-by: Julius Härtl <[email protected]>
juliusknorr
force-pushed
the
bugfix/10398/acceptance-files-changedir
branch
from
de1c5da to
16f6c07
Compare
juliusknorr
added
3. to review
juliusknorr
changed the title
|
Some other acceptance test is failing, i'll investigate. |
Signed-off-by: Julius Härtl <[email protected]>
juliusknorr
force-pushed
the
bugfix/10398/acceptance-files-changedir
branch
from
744c2ad to
1b4d502
Compare
|
Ok, I've adjusted the acceptance test to test for the original issue #1448 that the test was about. Moving to a single Failure at https://drone.nextcloud.com/nextcloud/server/9428/146 is unrelated. Ready for review. |
|
Wee, green acceptance tests! |
This should fix #10398. Once you switch to a different file list type like
FavoritesFileListthe detailsview element from the first file list has been removed from the DOM. Because of that it could not be shown when going back to the first file list.The underlying reason is that the current files app layout is based on one DetailsView per file list, we should change that for 15 maybe, but I would not introduce such a change now, since it might cause even more breakage.Edit: I have a branch that switches to use a single instance at 01f885e but as expected this will lead to various issues with the plugins that are registered for each file list.Turns out, we simply can run render to make sure that the sidebar view is updated in the dom when updating the details view from the file list. Thanks @skjnldsv 👍