-
Notifications
You must be signed in to change notification settings - Fork 49
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
frontend: fix mobile view of materialLists #3996
Conversation
Navigate to the list of materiallists in the NavBottombar. Add new view for the list of materiallists to navigate to the details of one materiallist. Rename the views/admin/(,Admin)MaterialLists component to avoid name clash with the new component. Add quick way to differentiate the target routes in the materialListRoute helper for the NavBottombar and NavTopbar. May have been introduced in ecamp#3841. Also add guard if camp is not loaded to materialListRoute.
⛔ Feature branch deployment currently inactive.If the PR is still open, you can add the |
@@ -3,7 +3,7 @@ Show all material lists for a camp on mobile | |||
--> |
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.
Update or delete the comment on the line above this
<template v-if="!isGuest" #title-actions> | ||
<DialogMaterialListCreate :camp="camp()"> | ||
<template #activator="{ on }"> | ||
<ButtonAdd class="mr-n2" height="32" v-on="on" | ||
>{{ $tc('global.button.create') }} | ||
</ButtonAdd> | ||
</template> | ||
</DialogMaterialListCreate> | ||
</template> |
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.
So you actively decided to keep this button in this view?
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 think this makes sense
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 the fix. I think my change requests can be done in the followup issue #3997
camp: campFromRoute(route), | ||
}), | ||
}, | ||
}, |
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 think material/lists
should move to the camp route object as a child. (But not material/all
as it needs the sidebar.
if (camp._meta.loading) return {} | ||
if (typeof materialListOrRoute === 'string') { | ||
return { | ||
name: 'material/overview', | ||
name: `material${materialListOrRoute}`, | ||
params: { campId: camp.id, campTitle: slugify(camp.title) }, | ||
query, | ||
} | ||
} |
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.
/all
and /lists
should probably move to campRoute and have a route name prefix "camp/material/$1"
Fixes #3899
Navigate to the list of materiallists in the NavBottombar. Add new view for the list of materiallists
to navigate to the details of one materiallist.
Rename the views/admin/(,Admin)MaterialLists component to avoid name clash with the new component.
Add quick way to differentiate the target routes in the materialListRoute helper for the NavBottombar and NavTopbar.
May have been introduced in #3841.
Also add guard if camp is not loaded to materialListRoute.