Sidebar panel extension #10111

kulmann · 2023-12-04T13:53:07Z

Description

This PR introduces a new extension point sidebarPanel, which allows apps to register additional panels for the right sidebar. It also extracts the right sidebar that was specific to the files app into a generic FileSideBar component which lives in the web-pkg package. The FileSideBar is then used in all viewer and editor apps so far to display a right sidebar with the files sidebar panels which were registered by the files app. I.e. panels live in web-app-files, but are used in any viewer/editor app as well.

Related Issue

Works towards Epic: Extension system #7930

Motivation and Context

Provide files app functionality like viewing file details, sharing, etc, directly in viewers and editors. Make it possible for viewer/editor apps to bring their own right sidebar panels and display them along the other files app sidebar panels.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

either use AppWrapper in preview app or embed the FileSideBar directly in the preview app
debug file info panel in apps (missing information, e.g. share indicators and last modified date)
double-/triple-check sidebar edge cases in files app regarding shares (e.g. sub-folder inside a share): is the share info loaded correctly?
find a place for the right sidebar toggle button in apps
details panel: disable previews in file details, the editor/app is BETTER than the preview ;-) only display the icon.
update file size and last modified date on save in editors :-)
fix panzoom-bug in preview app when navigating through images with open sidebar
fix sidebar width in apps

update-docs · 2023-12-04T13:53:11Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

packages/web-pkg/src/components/SideBar/SideBar.vue

kulmann · 2023-12-06T07:59:20Z

@janackermann @lookacat @JammingBen @dschmidt this PR is not quite done, but I'd like to ask for a critical review of three specific things (+ discussion together in a video call probably). This PR changes the SideBarPanel interface and exposes a new extension type for the panel. Hence I'd like to give it some extra brain power so that we don't publicly release a new extension point with maybe obvious issues I just didn't see...

The new SideBarPanelExtension interface lives in packages/web-pkg/src/composables/piniaStores/extensionRegistry.ts. Pretty minimalistic, should be ok, but just for the sake of completeness. This is the interface that needs to be implemented in order to register side bar panels for the right sidebar from any app/extension.

The Panel interface has been renamed to SideBarPanel in the course of making this an official extension point. It lives in packages/web-pkg/src/components/SideBar/types.ts. This is the interface where I'd like to ask you guys to look very closely if this has rough edges or even major issues. I decided to go for callbacks instead of getters or props with primitive types because that way we can get rid of the old style of having a generator function.

You can have a look at the new interface in action in packages/web-app-files/src/fileSideBars.ts where all right sidebar panels of the files app are defined. With one major difference: these right sidebar panels are now registered by the files app to be used in every other app which has a file context. That's already working for all the editors we have. It's somewhat broken in the preview app, working on that today. Just wanted you to see how a panel definition and registration looks like with the new interfaces.

There is one new component, which is the FileSideBar.vue. That is the one dynamically querying registered panels.

You can check out the PR and run it, it is already in quite a good shape...

within the files app the goal is that you don't recognize any difference ;-)
e.g. open a text file in the text editor. For the WIP state of the PR there is a dummy button in the topbar for opening and closing the right sidebar. Still need to figure out a good place for that... you should be able to do really just anything that also works in the files app (sharing, tagging, version management, etc).

There is a list of TODO items in the PR description. Please check there before you throw any missing features at me. ;-)

packages/web-app-preview/src/App.vue

packages/web-pkg/src/components/AppTemplates/AppWrapper.vue

AlexAndBear · 2023-12-06T09:49:05Z

Do you plan to have something similar like FileSideBar.vue and fileSideBars.ts for web-app-admin-settings as well?
so we will only have one or two files for the Sidebar here as well? I think that could make sense as the current Views are getting tooo big :(

AlexAndBear · 2023-12-06T09:56:45Z

About the correct place for the sidebar toggle in apps:
I'd go with a light transparent toggle button, that is located at the same position as the x button in the sidebar. If that is not an option, because it might look bad with OO or Collabroa, I think it's time to refactor the app-top-bar and give it a 3 dot menu. As it doesn't look so nice anyways, maybe toby can come up with some cool figma mockups

packages/web-app-files/src/fileSideBars.ts

packages/web-client/src/helpers/resource/types.ts

packages/web-pkg/src/components/SideBar/FileSideBar.vue

JammingBen · 2023-12-06T09:54:39Z

packages/web-pkg/src/components/SideBar/types.ts

+export interface SideBarPanelContext<P, T extends Item> {
+  parent?: P
+  items?: T[]
+}


Just some unfiltered thought process:

Do we need the space in the future? I'm a bit torn. On one hand, it's quite files-specific since we don't have a space in the admin-settings e.g. But then again, you could say the same about parent.

The only reason to add it here would be to have it available in isVisible and isRoot, right? Might be helpful if I want my panel to be visible in project spaces only. It could also save us from some of the not-so-nice route checks.

Trying to keep the generic scope, I added a root field to the context interface and set the space as root wherever possible. Not using it, yet, but I agree that it makes a whole lot of sense to have the space root available.

packages/web-pkg/src/composables/piniaStores/extensionRegistry.ts

packages/web-client/src/helpers/index.ts

AlexAndBear · 2023-12-06T10:03:38Z

packages/web-client/src/helpers/item.ts

@@ -0,0 +1,3 @@
+export interface Item {


Not a fan of the naming because it's so generic, even if its just there to extend existing interfaces

this interface indeed as generic as it can get, so I kind of like the name... do you have an idea for a better name? open for suggestions!
The reason why I introduced the interface is to be able to always require an id property on the items in a sidebar panel context.

packages/web-pkg/src/components/SideBar/FileSideBar.vue

JammingBen

LGTM 👍

There are still some occurrences of fileSideBars in the code base, we should deprecate/remove them in a follow-up.

sonarqubecloud · 2023-12-12T10:23:54Z

SonarCloud Quality Gate failed.

3 Bugs
0 Vulnerabilities
1 Security Hotspot
179 Code Smells

48.6% Coverage
0.5% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

* feat: introduce sidebar panels as extension point --------- Co-authored-by: Dominik Schmidt <[email protected]> Co-authored-by: Jannik Stehle <[email protected]>

dschmidt · 2023-12-12T11:20:27Z

😍 😍 😍

* feat: introduce sidebar panels as extension point --------- Co-authored-by: Dominik Schmidt <[email protected]> Co-authored-by: Jannik Stehle <[email protected]>

kulmann commented Dec 4, 2023

View reviewed changes

packages/web-pkg/src/components/SideBar/SideBar.vue Outdated Show resolved Hide resolved

kulmann force-pushed the sidebar-panel-extension branch from 07c5c77 to 3cadbd6 Compare December 4, 2023 14:45

AlexAndBear requested changes Dec 6, 2023

View reviewed changes

packages/web-app-preview/src/App.vue Outdated Show resolved Hide resolved

packages/web-pkg/src/components/AppTemplates/AppWrapper.vue Outdated Show resolved Hide resolved

JammingBen reviewed Dec 6, 2023

View reviewed changes

AlexAndBear reviewed Dec 6, 2023

View reviewed changes

packages/web-pkg/src/composables/piniaStores/extensionRegistry.ts Show resolved Hide resolved

AlexAndBear reviewed Dec 6, 2023

View reviewed changes

packages/web-client/src/helpers/index.ts Outdated Show resolved Hide resolved

AlexAndBear requested changes Dec 6, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

kulmann force-pushed the sidebar-panel-extension branch from fbb4d68 to 294647c Compare December 6, 2023 15:06

dschmidt mentioned this pull request Dec 6, 2023

feat: retrieve audio and location metadata #10125

Merged

9 tasks

kulmann force-pushed the sidebar-panel-extension branch 2 times, most recently from 86bf0ab to 4200b4d Compare December 7, 2023 13:14

dschmidt force-pushed the sidebar-panel-extension branch from 7869863 to fff2c86 Compare December 7, 2023 14:55

This comment was marked as outdated.

Sign in to view

kulmann force-pushed the sidebar-panel-extension branch 2 times, most recently from 45091a4 to 73f3cca Compare December 11, 2023 11:35

kulmann added 10 commits December 11, 2023 21:59

feat: introduce sidebar panels as extension point

7c48653

refactor: move files app sidebar into web-pkg

20e0e99

feat: generic file sidebar in AppWrapper

333d089

fix: displayed info in details panel within apps

2ebbbee

feat: add FileSideBar to preview app

779eacf

refactor: move panel filtering to SideBar

bf8a703

refactor: rename isEnabled to isVisible in SideBarPanel

f650be9

refactor: rename files scope to resource

434a819

refactor: rename open and sideBarOpen to isOpen and isSideBarOpen

b4ef359

refactor: use unknown instead of any

c163be7

dschmidt and others added 10 commits December 11, 2023 21:59

feat(sidebarPanel): Make content use all available vertical space

8be2145

feat: add onPanelActive subscription helper

c28c5b6

fix: hide useless items in details panel for personal space root

cb084cb

refactor: types

44d23d9

fix: types

39754d9

test: adjust unit tests for FileSideBar

f665c97

fix: wrong types

dc91a43

style: run linter

d3a74fe

test: fix unit tests

1c60350

test: fix acceptance and e2e tests

476dba5

kulmann force-pushed the sidebar-panel-extension branch from b3d4ee4 to 6ef6be1 Compare December 11, 2023 21:04

docs: add changelog item

987513e

kulmann force-pushed the sidebar-panel-extension branch from 712e6e4 to 705a08a Compare December 11, 2023 21:59

kulmann marked this pull request as ready for review December 11, 2023 22:00

AlexAndBear approved these changes Dec 12, 2023

View reviewed changes

kulmann self-assigned this Dec 12, 2023

JammingBen approved these changes Dec 12, 2023

View reviewed changes

feat: move sidebar toggle button into the topbar

8c1a51f

kulmann force-pushed the sidebar-panel-extension branch from 705a08a to 8c1a51f Compare December 12, 2023 10:01

fix: enforce sidebar width

53c4871

kulmann merged commit 673783c into master Dec 12, 2023
3 of 4 checks passed

delete-merged-branch bot deleted the sidebar-panel-extension branch December 12, 2023 10:50

ownclouders pushed a commit that referenced this pull request Dec 12, 2023

Sidebar panel extension (#10111)

66fbbbe

* feat: introduce sidebar panels as extension point --------- Co-authored-by: Dominik Schmidt <[email protected]> Co-authored-by: Jannik Stehle <[email protected]>

This was referenced Dec 12, 2023

fix: preview app while sidebar being opened #10152

Merged

Streamline resource and share id types #9921

Closed

micbar mentioned this pull request Dec 12, 2023

[full-ci] docs: prepare changelog, bump version owncloud/ocis#7951

Merged

12 tasks

AlexAndBear pushed a commit that referenced this pull request Dec 13, 2023

Sidebar panel extension (#10111)

42fe6c2

* feat: introduce sidebar panels as extension point --------- Co-authored-by: Dominik Schmidt <[email protected]> Co-authored-by: Jannik Stehle <[email protected]>

micbar mentioned this pull request Dec 15, 2023

Release 5.0.0 owncloud/ocis#7940

Closed

71 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sidebar panel extension #10111

Sidebar panel extension #10111

kulmann commented Dec 4, 2023 •

edited by JammingBen

Loading

update-docs bot commented Dec 4, 2023

kulmann commented Dec 6, 2023 •

edited

Loading

AlexAndBear commented Dec 6, 2023

AlexAndBear commented Dec 6, 2023 •

edited

Loading

JammingBen Dec 6, 2023

kulmann Dec 6, 2023

AlexAndBear Dec 6, 2023

kulmann Dec 6, 2023

This comment was marked as outdated.

This comment was marked as outdated.

JammingBen left a comment

sonarqubecloud bot commented Dec 12, 2023

dschmidt commented Dec 12, 2023

Sidebar panel extension #10111

Sidebar panel extension #10111

Conversation

kulmann commented Dec 4, 2023 • edited by JammingBen Loading

Description

Related Issue

Motivation and Context

Types of changes

Checklist:

Open tasks:

update-docs bot commented Dec 4, 2023

kulmann commented Dec 6, 2023 • edited Loading

AlexAndBear commented Dec 6, 2023

AlexAndBear commented Dec 6, 2023 • edited Loading

JammingBen Dec 6, 2023

Choose a reason for hiding this comment

kulmann Dec 6, 2023

Choose a reason for hiding this comment

AlexAndBear Dec 6, 2023

Choose a reason for hiding this comment

kulmann Dec 6, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

JammingBen left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 12, 2023

dschmidt commented Dec 12, 2023

kulmann commented Dec 4, 2023 •

edited by JammingBen

Loading

kulmann commented Dec 6, 2023 •

edited

Loading

AlexAndBear commented Dec 6, 2023 •

edited

Loading