-
Notifications
You must be signed in to change notification settings - Fork 156
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
fix ocis-1043 #4444
fix ocis-1043 #4444
Conversation
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.
I guess cloning the whole items
instead of one by one wouldn't make a real difference here with performance, right? Since I guess there won't ever be hundreds of nav items 🤷
show loading screen while waiting for all required data from backend check for route path instead of name in ADD_NAV_ITEM fix session persist and add only what is required
a45f37e
to
2cee264
Compare
@LukasHirt, yes its more costy to do it one by one (but not much it's a low level api). The problem if we do
the result would for n be [99,2,3] and o is still [1, 2, 3] but if we do
we will mutate the original object in o objects in arrays get refferenced and not deep copied. |
src/Phoenix.vue
Outdated
}" | ||
uk-height-viewport | ||
> | ||
<oc-spinner size="xxxlarge" :aria-label="$gettext('Loading')" class="uk-position-center" /> |
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.
From my personal feeling the spinner is too big. I would choose xlarge
size and border sizes in the css class 10px
.
@@ -32,7 +32,9 @@ const mutations = { | |||
ADD_NAV_ITEM(state, { extension, navItem }) { | |||
const dynamicNavItems = { ...state.dynamicNavItems } | |||
dynamicNavItems[extension] = dynamicNavItems[extension] || [] | |||
const index = dynamicNavItems[extension].findIndex(item => item.name === navItem.name) | |||
const index = dynamicNavItems[extension].findIndex( | |||
item => item.route.path === navItem.route.path |
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.
Thanks for spotting this ❤️
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.
👍
partially fixes: owncloud/ocis#1043
fixes: owncloud/ocis#884