-
Notifications
You must be signed in to change notification settings - Fork 157
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
[full-ci] move sidebar state into views #7559
Conversation
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. |
@@ -37,9 +37,9 @@ export default { | |||
}, | |||
inject: ['displayedItem'], | |||
props: { | |||
isContentDisplayed: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prop was unused
e7254d6
to
7820836
Compare
} | ||
}, | ||
setup() { | ||
const { fileListHeaderY, selectedResourcesIds, selectedResources } = useResourcesViewDefaults< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decided to remove the useResourcesViewDefaults
composable here, because that one now subscribes to some event bus topics - it should only be used in a view (once per view). Additionally I moved the variables and methods related to selected resources
into its own composable (which is then spreaded into the useResourcesViewDefaults composable) so that we can continue to use that here. Remaining composable refs sideBarOpen
(new) and fileListHeaderY
(existing) have been moved to props and need to be passed down by the shared with me view.
:available-panels="availablePanels" | ||
:sidebar-accordions-warning-message="sidebarAccordionsWarningMessage" | ||
:warning-message="warningMessage" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
felt that this needs renaming ;-)
ref="sidebar" | ||
class="files-side-bar" | ||
tabindex="-1" | ||
:sidebar-active-panel="sidebarActivePanel" | ||
:open="open" | ||
:active-panel="activePanel" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namespacing the props of the sidebar component seemed unnecessary - renamed.
7820836
to
417ac3d
Compare
@@ -72,27 +77,6 @@ export const useResourcesViewDefaults = <T, TT, TU extends any[]>( | |||
fileList.accentuateItem(payload.id) | |||
}) | |||
|
|||
const selectedResources: WritableComputedRef<Resource[]> = computed({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved into new composable useSelectedResources
// return hardcoded `actions-item` in all cases once we have them. | ||
await this.openSidebarWithPanel( | ||
$_showActions_trigger() { | ||
// we don't have details in the trashbin, yet. the actions panel is the default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some more context to the explanation (unrelated to this PR otherwise)
Results for oCISSharingPublic2 https://drone.owncloud.com/owncloud/web/27963/69/1 |
417ac3d
to
7f2fbe9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Sidebar still behaves as it should as far as I can see.
9c106b3
to
bbb87c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Kudos, SonarCloud Quality Gate passed! |
Description
This PR gets rid of the
sidebar
vuex module in the files app, in favour of a composable, which has been added to theuseResourcesViewDefaults
composable as well. This clears up some "magic state" of the sidebar component by wiring the sidebar state management into views, which then pass down thesideBarOpen
andsideBarActivePanel
refs to whichever child component needs them.Motivation and Context
Works towards a clear hierarchy in views.
Types of changes
Checklist: