enhancement: context aware preview app media rendering #9882
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
I don't like this kind of specific route handling in a concrete app implementation.
The app loads the files via loadFolderForFileContext, that function should be patched instead to handle the context correctly and load the correct folder.
Being able to implement that was one of the reasons to introduce the folder loaders. The proper implementation for this change would be to use those in loadFolderForContexr imho
|
@tbsbdr i talked to @dschmidt and we discussed some use cases: Scenario: a user has 5 single file public links, the user navigates to the shares, shared via link panel. The user clicks one file from the list. a) the user can browse through (preview) all 5 files, no matter in which parent folder the individual file is b) the user gets only 1 file in the preview, no navigation, no nothing (the user has to close the preview and select another file to see it.) same applies for search, favorite, …. b) is currently implemented in that pr, as described in the issue, a) sounds more useful to me but involves more work and maybe more network traffic (search folder loader, favorites loader, …), but this must be evaluated to give a valuable answer what do you say, a or b? |
|
I'd argue browsing through multiple single file shares might be a bit odd, but especially for favorites I think it makes lot of sense to be able to browse. Anyhow I think the behavior should be implemented in the composable so all apps loading a context folder share the same behavior. Currently it's only the preview app but I like to be prepared for additional ones |
I agree, have it there makes totally sense to me too, but then the composable has to have knowledge about the app context too!? if we decide to have that „single file“ behavior globally, the composable is the best place for it. Let’s discuss that tomorrow after my dentist appointment :( |
|
The composable has access to file context, file context contains the route which is exactly what you need here. I think it comes all together pretty nicely. I don't insist on single file or folder loading behavior for specific cases, in fact we can go through all scenarios and define behavior individually. I just insist on handling it through the composable to make it consistent and keep it separated from app logic but yes, we can totally discuss tomorrow or whenever you are feeling better |
|
@JammingBen, @dschmidt and i discussed the behavior again today and came to the conclusion that we prefer solution
in that case its not a bugfix anymore and should be part of our regular sprint work and needs a user story. cc.: @tbsbdr. i keep that open for later reference as a draft. |
Description
Until now, the preview app had always displayed all images,
regardless of what the user had selected or where the request came from.
Of course, permissions have been taken into account so far,
but the behavior was still wrong in certain cases.
In the following scenarios, the app displayed all media from the same folder:
shared via linkpanel,clicked the single shared resource.
shared with otherspanel, clicked the single shared resource.clicked on one from that list.
now the behavior is changed and the preview app takes the referer into consideration and displays
only the selected resource in one of the above cases.
not sure if its the perfect way to have a explicit list of those context's, maybe a exclude list would be more (everything expect the direct space referer)!? what do you think?
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Screen.Recording.2023-10-29.at.15.08.42.mp4
Types of changes
Checklist: