feature/projects-all-the-way-down #3323
|
@ -19,23 +19,10 @@
|
|||
}"
|
||||
>
|
||||
<template #item="{element: project}">
|
||||
dpschen marked this conversation as resolved
Outdated
|
||||
<li
|
||||
class="list-menu loader-container is-loading-small"
|
||||
:class="{'is-loading': projectUpdating[project.id]}"
|
||||
:data-project-id="project.id"
|
||||
>
|
||||
<ProjectsNavigationItem
|
||||
:project="project"
|
||||
:can-collapse="childProjects[project.id]?.length > 0"
|
||||
:is-collapsed="collapsedProjects[project.id] || false"
|
||||
@collapse="collapsedProjects[project.id] = !collapsedProjects[project.id]"
|
||||
/>
|
||||
<ProjectsNavigation
|
||||
v-if="!collapsedProjects[project.id]"
|
||||
v-model="childProjects[project.id]"
|
||||
:can-edit-order="true"
|
||||
/>
|
||||
</li>
|
||||
<ProjectsNavigationItem
|
||||
:project="project"
|
||||
:is-loading="projectUpdating[project.id]"
|
||||
/>
|
||||
</template>
|
||||
</draggable>
|
||||
dpschen
commented
Since this block doesn't have a headline it shouldn't be a Since this block doesn't have a headline it shouldn't be a `<section>`. Maybe use `<nav>` instead (nesting is allowed!)
dpschen
commented
Move this whole block in a new Move this whole block in a new `ProjectNavigationItem.vue` component. Reduces also the whole complexity with `childProjects[p.id]` because we can pass only the project.
konrad
commented
Done > Move this whole block in a new ProjectNavigationItem.vue component. Reduces also the whole complexity with childProjects[p.id] because we can pass only the project.
Done
konrad
commented
Shouldn't a Shouldn't a `nav` hold multiple navigation items?
dpschen
commented
Yes! Sry I misread the position, where the
Okay you moved now only the item without the list below inside.
This whole block can then be simplified:
Because we can save the collapsed state inside each item we don't need to manage a list anymore.
> Shouldn't a `nav` hold multiple navigation items?
Yes! Sry I misread the position, where the `<section>` is.
> Done
Okay you moved now only the item without the list below inside.
What i meant was:
- Move the complete `<li>` inside `ProjectsNavigationItem.vue`.
- `ProjectsNavigation.vue` is then used inside ProjectsNavigationItem
This whole block can then be simplified:
```ts
const collapsedProjects = ref<{ [id: IProject['id']]: boolean }>({})
const availableProjects = ref<IProject[]>([])
const childProjects = ref<{ [id: IProject['id']]: boolean }>({})
watch(
() => props.modelValue,
projects => {
availableProjects.value = projects || []
projects?.forEach(p => {
collapsedProjects.value[p.id] = false
childProjects.value[p.id] = projectStore.getChildProjects(p.id)
.sort((a, b) => a.position - b.position)
})
},
{immediate: true},
)
```
Because we can save the collapsed state inside each item we don't need to manage a list anymore.
```ts
const childProjectsOpen = ref(true)
// if getChildProjects returns the list sorted by position by default we wouldn't even need this computed
const childProjects = computed(() => {
const projects = projectStore.getChildProjects(p.id)
return projects.sort((a, b) => a.position - b.position)
})
```
dpschen
commented
So we don't even need the It's totally fine to not group the buttons etc because they are already grouped by the So we don't even need the `<section>` then and can instead use the `<li>`.
It's totally fine to not group the buttons etc because they _are_ already grouped by the `<li>` they are in. The `ProjectsNavigation` component would be the last child insie `ProjectsNavigationItem`
konrad
commented
That makes sense. I've moved most of the logic over, as you suggested.
We actually need this (or another element) because the That makes sense. I've moved most of the logic over, as you suggested.
> So we don't even need the `<section>` then and can instead use the `<li>`.
We actually need this (or another element) because the `section` is a flexbox container for the project title and related buttons. We can't use the `li` as the flexbox container because the ProjectsNavigation for the child projects needs to stay below the project title etc. If it was in the same flexbox container it would get pushed to the right.
|
||||
</template>
|
||||
|
@ -68,18 +55,11 @@ const projectStore = useProjectStore()
|
|||
|
||||
// Vue draggable will modify the projects list as it changes their position which will not work on a prop.
|
||||
// Hence, we'll clone the prop and work on the clone.
|
||||
const collapsedProjects = ref<{ [id: IProject['id']]: boolean }>({})
|
||||
const availableProjects = ref<IProject[]>([])
|
||||
dpschen
commented
Is this even necessary if we use Is this even necessary if we use `modelValue` instead of `v-model` for the draggable?
konrad
commented
`v-model` is required, using `modelValue` for the draggable component does not work.
|
||||
const childProjects = ref<{ [id: IProject['id']]: boolean }>({})
|
||||
watch(
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
Didn't we just remove this condition, so that also one can also adjust settings of favorited lists? Didn't we just remove this condition, so that also one can also adjust settings of favorited lists?
konrad
commented
The condition is already in main: https://kolaente.dev/vikunja/frontend/src/branch/main/src/components/home/navigation.vue#L132 But editing favorites works just fine. This is about every project which actually exists, so no shared filters for example. The condition is already in main: https://kolaente.dev/vikunja/frontend/src/branch/main/src/components/home/navigation.vue#L132
But editing favorites works just fine. This is about every project which actually exists, so no shared filters for example.
dpschen
commented
My bad I confused favorite projects with the 'Favorite' project. My bad I confused favorite projects with the 'Favorite' project.
|
||||
() => props.modelValue,
|
||||
projects => {
|
||||
availableProjects.value = projects || []
|
||||
projects?.forEach(p => {
|
||||
collapsedProjects.value[p.id] = false
|
||||
childProjects.value[p.id] = projectStore.getChildProjects(p.id)
|
||||
.sort((a, b) => a.position - b.position)
|
||||
})
|
||||
},
|
||||
{immediate: true},
|
||||
)
|
||||
|
|
|
@ -1,49 +1,60 @@
|
|||
<template>
|
||||
<section>
|
||||
<BaseButton
|
||||
v-if="canCollapse"
|
||||
@click="emit('collapse')"
|
||||
class="collapse-project-button"
|
||||
>
|
||||
<icon icon="chevron-down" :class="{ 'project-is-collapsed': isCollapsed }"/>
|
||||
</BaseButton>
|
||||
<span class="collapse-project-button-placeholder" v-else></span>
|
||||
<BaseButton
|
||||
:to="{ name: 'project.index', params: { projectId: project.id} }"
|
||||
class="list-menu-link"
|
||||
:class="{'router-link-exact-active': currentProject.id === project.id}"
|
||||
>
|
||||
<li
|
||||
class="list-menu loader-container is-loading-small"
|
||||
:class="{'is-loading': isLoading}"
|
||||
:data-project-id="project.id"
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
Set from outside, since this id is related to the sorting. Set from outside, since this id is related to the sorting.
konrad
commented
Done Done
|
||||
>
|
||||
<section>
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
Replace section with We'll add correct semantics here later (e.g. https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-navigation/). Replace section with `<div>`.
We'll add correct semantics here later (e.g. https://www.w3.org/WAI/ARIA/apg/patterns/menubar/examples/menubar-navigation/). `<section>` is not correct though, since there is no headline.
konrad
commented
Done Done
|
||||
<BaseButton
|
||||
v-if="childProjects?.length > 0"
|
||||
@click="childProjectsOpen = !childProjectsOpen"
|
||||
class="collapse-project-button"
|
||||
>
|
||||
<icon icon="chevron-down" :class="{ 'project-is-collapsed': !childProjectsOpen }"/>
|
||||
</BaseButton>
|
||||
<span class="collapse-project-button-placeholder" v-else></span>
|
||||
<BaseButton
|
||||
:to="{ name: 'project.index', params: { projectId: project.id} }"
|
||||
class="list-menu-link"
|
||||
:class="{'router-link-exact-active': currentProject.id === project.id}"
|
||||
>
|
||||
<span class="icon menu-item-icon handle">
|
||||
dpschen
commented
Fix indention Fix indention
dpschen
commented
Pass 'handle class name from parent => separate concerns / source of truth Pass 'handle class name from parent => separate concerns / source of truth
konrad
commented
But the indention is correct? But the indention is correct?
konrad
commented
Can you explain that a little more? > Pass 'handle class name from parent => separate concerns / source of truth
Can you explain that a little more?
dpschen
commented
The The `handle` selector is used in the child. Currently we define it in the parent. We should pass this information down to the child. Might also be via creating a slot in the child where we put in the handle.
|
||||
<icon icon="grip-lines"/>
|
||||
</span>
|
||||
<ColorBubble
|
||||
v-if="project.hexColor !== ''"
|
||||
:color="project.hexColor"
|
||||
class="mr-1"
|
||||
/>
|
||||
<span class="list-menu-title">{{ getProjectTitle(project) }}</span>
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-if="project.id > 0"
|
||||
class="favorite"
|
||||
:class="{'is-favorite': project.isFavorite}"
|
||||
@click="projectStore.toggleProjectFavorite(project)"
|
||||
>
|
||||
<icon :icon="project.isFavorite ? 'star' : ['far', 'star']"/>
|
||||
</BaseButton>
|
||||
<ProjectSettingsDropdown class="menu-list-dropdown" :project="project" v-if="project.id > 0">
|
||||
<template #trigger="{toggleOpen}">
|
||||
<BaseButton class="menu-list-dropdown-trigger" @click="toggleOpen">
|
||||
<icon icon="ellipsis-h" class="icon"/>
|
||||
</BaseButton>
|
||||
</template>
|
||||
</ProjectSettingsDropdown>
|
||||
<span class="list-setting-spacer" v-else></span>
|
||||
</section>
|
||||
<ColorBubble
|
||||
v-if="project.hexColor !== ''"
|
||||
:color="project.hexColor"
|
||||
class="mr-1"
|
||||
/>
|
||||
<span class="list-menu-title">{{ getProjectTitle(project) }}</span>
|
||||
</BaseButton>
|
||||
<BaseButton
|
||||
v-if="project.id > 0"
|
||||
class="favorite"
|
||||
:class="{'is-favorite': project.isFavorite}"
|
||||
@click="projectStore.toggleProjectFavorite(project)"
|
||||
>
|
||||
<icon :icon="project.isFavorite ? 'star' : ['far', 'star']"/>
|
||||
</BaseButton>
|
||||
<ProjectSettingsDropdown class="menu-list-dropdown" :project="project" v-if="project.id > 0">
|
||||
<template #trigger="{toggleOpen}">
|
||||
<BaseButton class="menu-list-dropdown-trigger" @click="toggleOpen">
|
||||
<icon icon="ellipsis-h" class="icon"/>
|
||||
</BaseButton>
|
||||
</template>
|
||||
</ProjectSettingsDropdown>
|
||||
<span class="list-setting-spacer" v-else></span>
|
||||
</section>
|
||||
<ProjectsNavigation
|
||||
v-if="childProjectsOpen"
|
||||
dpschen
commented
Use Use `Expandable` component for this.
konrad
commented
This seems to completly break the styling. I changed it to match the selectors but it still does not work. Not sure what to do about this.- This seems to completly break the styling. I changed it to match the selectors but it still does not work. Not sure what to do about this.-
dpschen
commented
I created an example how to use this in I created an example how to use this in https://kolaente.dev/vikunja/frontend/commit/51e29af010defc5f6c46f85dbb7311904a7d40e1. I was unsure which parts parts should be dynamically be filled (via the `open` prop) or static (not rendering the `Expandable` at all via `v-if`).
|
||||
v-model="childProjects"
|
||||
:can-edit-order="true"
|
||||
/>
|
||||
</li>
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import {computed} from 'vue'
|
||||
import {computed, watch, ref} from 'vue'
|
||||
import {useProjectStore} from '@/stores/projects'
|
||||
import {useBaseStore} from '@/stores/base'
|
||||
|
||||
|
@ -53,18 +64,24 @@ import BaseButton from '@/components/base/BaseButton.vue'
|
|||
import ProjectSettingsDropdown from '@/components/project/project-settings-dropdown.vue'
|
||||
import {getProjectTitle} from '@/helpers/getProjectTitle'
|
||||
import ColorBubble from '@/components/misc/colorBubble.vue'
|
||||
import ProjectsNavigation from '@/components/home/ProjectsNavigation.vue'
|
||||
|
||||
defineProps<{
|
||||
const props = defineProps<{
|
||||
project: IProject,
|
||||
dpschen
commented
Define default types or handle undefined defaults. E.g. Define default types or handle undefined defaults. E.g. `level` might be `undefined`.
Could it be that ts doesn't display errors in the editor for you?
konrad
commented
Added.
Looks like it 🤔 Added.
> Could it be that ts doesn't display errors in the editor for you?
Looks like it 🤔
|
||||
isCollapsed: boolean,
|
||||
canCollapse: boolean,
|
||||
isLoading?: boolean,
|
||||
}>()
|
||||
|
||||
const emit = defineEmits(['collapse'])
|
||||
|
||||
const projectStore = useProjectStore()
|
||||
const baseStore = useBaseStore()
|
||||
const currentProject = computed(() => baseStore.currentProject)
|
||||
|
||||
const childProjectsOpen = ref(true)
|
||||
|
||||
const childProjects = computed(() => {
|
||||
return projectStore.getChildProjects(props.project.id)
|
||||
.sort((a, b) => a.position - b.position)
|
||||
})
|
||||
|
||||
</script>
|
||||
|
||||
<style lang="scss" scoped>
|
||||
|
|
Use long var name. I thought for a bit that for
<draggable>
you can define the type of dom node of the child item viaelement
.Done.