-
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
Implement spaces list #6199
Implement spaces list #6199
Conversation
b39d0c8
to
3f86aa9
Compare
@dragotin how's the |
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.
The static nav item is an issue. We need to have an enabled spaces capability. IMO we should add an optional isEnabled
callback to nav items, similar to the file action mixins. Could you look into that? Needs the store in the param object so that you can check the capabilities.
Results for oC10SharingExternalRoot https://drone.owncloud.com/owncloud/web/21530/41/1
|
Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/21540/15/1
|
Results for oCISSharingInternal3 https://drone.owncloud.com/owncloud/web/21543/59/1
|
Results for oC10SharingPublic2 https://drone.owncloud.com/owncloud/web/21543/38/1
|
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.
Some nitpicks. ;-)
Please rebase to current master. Icons/ODS update has been merged to web master.
In index.js
of the files app you can append personal/
to the route of the All files
nav item. That will fix the duplicate highlighting of active nav items in the sidebar.
Makes sense, yes.
Yes, planned to be removed this week. 😉 basically anything that is |
9b27d85
to
f543b90
Compare
Addressed your comments 👍 Two questions left:
|
Awesome, looks better now. 👍
I hope that we can use the same AppBar for spaces as for the other views, but it will probably need some restructuring. ;-) For the overview of the spaces I think we can hide it completely. |
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.
found some small things.
I would love to see some e2e tests as part of this pr.
cleanup component dom abstract libre-graph stubs into temporary sdk
add dedicated client package and wrap libre-graph-api
revert removal of files app root redirect
a572f05
to
fa689f3
Compare
Kudos, SonarCloud Quality Gate passed! |
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.
Awesome! 💪
One thing that I found is, that the spaces are neither sorted by created time, nor by name. Not an issue for this PR, but please open a ticket about default sorting or simply sort by name for now. |
"license": "AGPL-3.0", | ||
"main": "src/index.ts", | ||
"scripts": { | ||
"generate-openapi": "rm -rf src/generated && docker run --rm -v \"${PWD}/src:/local\" openapitools/openapi-generator-cli generate -i https://raw.githubusercontent.com/owncloud/libre-graph-api/main/api/openapi-spec/v0.0.yaml -g typescript-axios -o /local/generated" |
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.
why not using this?
see https://github.com/owncloud/libre-graph-api/blob/main/.drone.star#L17-L21
I've created #6253, since we really want to merge this now as the pipeline is currently green for once :D |
Sorting should be a task for the API, this feature is yet not there, so we just wait until it is done and fix this in the next PR 😇 |
Yes, of course something for the API. I meant it as a mitigation for the status quo. Which is a oneliner. ;-) if sorting in the backend happens soon then it's fine. |
Description
Adds an entry point to access all available spaces for the user.
Related Issue
Screenshots (if appropriate):
Types of changes
Checklist:
Open tasks: