-
Notifications
You must be signed in to change notification settings - Fork 168
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
Add space sorting options: size
and mdate
#8675
Conversation
167738d
to
41dd3a8
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
@@ -51,13 +51,15 @@ | |||
v-oc-tooltip="showSpaceMemberLabel" | |||
:aria-label="showSpaceMemberLabel" | |||
appearance="raw" | |||
@click="openSidebarSharePanel(resource)" | |||
@click="openSidebarSharePanel(resource as SpaceResource)" |
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.
Would be nice to figure out how to type slots :-D
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.
Apparently you can type the context variables directly, see 299192a. I would like to use SpaceActionOptions
here, but the import gets removed by the linter... :(
Edit: nvm, I un-did that commit because that's not what we want.
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.
Turns out IntelliJ is smart enough to deduce the type from the slot declaration, vue-tsc apparently isn't... anyhow this is the correct fix, as resource-tiles component just deals with resources and that's why we need to downcast here.
Would be great if Vue had support for generic components 👀
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.
haha +1 for generic components 😅
299192a
to
13f232c
Compare
Results for e2e-tests oC10 https://drone.owncloud.com/owncloud/web/34010/11/1 💥 To see the trace, please open the link in the console ...
npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/34010/tracing/alice-can-share-this-weeks-meal-plan-with-all-parents-alice-2023-3-22-02-48-35.zipnpx playwright show-trace https://cache.owncloud.com/public/owncloud/web/34010/tracing/alice-can-share-this-weeks-meal-plan-with-all-parents-brian-2023-3-22-02-49-03.zipnpx playwright show-trace https://cache.owncloud.com/public/owncloud/web/34010/tracing/alice-can-share-this-weeks-meal-plan-with-all-parents-carol-2023-3-22-02-49-09.zip |
@@ -2,6 +2,7 @@ import { ref, Ref, computed, unref, isRef } from 'vue' | |||
import { MaybeRef, ReadOnlyRef } from 'web-pkg/src/utils' | |||
import { useRouteName, useRouteQueryPersisted, QueryValue } from 'web-pkg/src/composables' | |||
import { SortConstants } from './constants' | |||
import { useRouter } from 'vue-router' |
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.
Are we sure this always works? I had problems with this composable in the past and no time to debug
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.
Ohh good catch! I also had problems, I'll change that.
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.
out of curiosity, what kind of problems?
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.
Not returning our router instance or even: no instance at all.
As I said I haven't debugged it, so I can't tell you what exactly is going on.
@@ -51,13 +51,15 @@ | |||
v-oc-tooltip="showSpaceMemberLabel" | |||
:aria-label="showSpaceMemberLabel" | |||
appearance="raw" | |||
@click="openSidebarSharePanel(resource)" | |||
@click="openSidebarSharePanel(resource as SpaceResource)" |
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.
Turns out IntelliJ is smart enough to deduce the type from the slot declaration, vue-tsc apparently isn't... anyhow this is the correct fix, as resource-tiles component just deals with resources and that's why we need to downcast here.
Would be great if Vue had support for generic components 👀
13f232c
to
461e2de
Compare
SonarCloud Quality Gate failed. |
Related Issue
sortBy
andsortDir
both change #8409Screenshots (if appropriate):
Types of changes