fix: Always apply fullscreen styling to sidebar#39378
Conversation
ShGKme
left a comment
There was a problem hiding this comment.
I like this solution!
I didn't finish that PR in viewer because didn't find a good solution from the viewer side but didn't want to modify the server part :D
But this is definitely better.
Unfortunately, it still doesn't work in one case:
- Open files
- Open sidebar
- Open viewer
It works in combination with my PR in viewer, but with a 0.2s transition...
ShGKme
left a comment
There was a problem hiding this comment.
Approved, regardless viewer, IMO it is better to have correct fullscreen state in all sidebar states in files.
Also, I'd consider to make fullscreen mode NcAppSidebar component feature, so we would be able to use it in any apps and have better sidebar integration in apps (for example, having shared fullscreen / resize events).
jancborchardt
left a comment
There was a problem hiding this comment.
@juliushaertl in your screen recording the viewer and sidebar open and close visibly at slightly different times when clicking the close "x" and the file name respectively. Is this something new based on the approach? (Maybe it’s also just because of the screen recording.)
|
This is already the case before this PR, I'm purely targeting the weird spacing issue here. |
jancborchardt
left a comment
There was a problem hiding this comment.
Looks good design-wise.
Signed-off-by: Julius Härtl <[email protected]>
f2400ff to
ac13226
Compare
|
/compile |
Signed-off-by: nextcloud-command <[email protected]>

Summary
Make sure that the full screen styling is always applied to the sidebar even if the loading hasn't finished yet.
Fixes a small UI glitch where a small space was left between the viewer modal and the sidebar.
This is a different approach I came up before stumbling over nextcloud/viewer#1711 so I'd appreciate feedback especially from @ShGKme on this. It seems to also cover the drawback that the viewer PR has.
Before
After
Screen.Recording.2023-07-13.at.22.14.29.mov
TODO
Checklist