feature/projects-all-the-way-down #3323

Merged
konrad merged 123 commits from feature/projects-all-the-way-down into main 2023-05-30 10:09:40 +00:00
Showing only changes of commit a0d39e6081 - Show all commits

View File

@ -18,45 +18,45 @@
]
}"
>
<template #item="{element: p}">
<template #item="{element: project}">
dpschen marked this conversation as resolved Outdated

Use long var name. I thought for a bit that for <draggable> you can define the type of dom node of the child item via element.

Use long var name. I thought for a bit that for `<draggable>` you can define the type of dom node of the child item via `element`.

Done.

Done.
<li
class="list-menu loader-container is-loading-small"
:class="{'is-loading': projectUpdating[p.id]}"
:data-project-id="p.id"
:class="{'is-loading': projectUpdating[project.id]}"
:data-project-id="project.id"
>
<section>

Since this block doesn't have a headline it shouldn't be a <section>. Maybe use <nav> instead (nesting is allowed!)

Since this block doesn't have a headline it shouldn't be a `<section>`. Maybe use `<nav>` instead (nesting is allowed!)

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.

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.

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

> 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

Shouldn't a nav hold multiple navigation items?

Shouldn't a `nav` hold multiple navigation items?

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:

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.

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)
})
> 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) }) ```

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

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`

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.

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.
<BaseButton
v-if="childProjects[p.id]?.length > 0"
@click="collapsedProjects[p.id] = !collapsedProjects[p.id]"
v-if="childProjects[project.id]?.length > 0"
@click="collapsedProjects[project.id] = !collapsedProjects[project.id]"
class="collapse-project-button"
>
<icon icon="chevron-down" :class="{ 'project-is-collapsed': collapsedProjects[p.id] }"/>
<icon icon="chevron-down" :class="{ 'project-is-collapsed': collapsedProjects[project.id] }"/>
</BaseButton>
<span class="collapse-project-button-placeholder" v-else></span>
<BaseButton
:to="{ name: 'project.index', params: { projectId: p.id} }"
:to="{ name: 'project.index', params: { projectId: project.id} }"
class="list-menu-link"
:class="{'router-link-exact-active': currentProject.id === p.id}"
:class="{'router-link-exact-active': currentProject.id === project.id}"
>
<span class="icon menu-item-icon handle">
<icon icon="grip-lines"/>
</span>
<ColorBubble
v-if="p.hexColor !== ''"
:color="p.hexColor"
v-if="project.hexColor !== ''"
:color="project.hexColor"
class="mr-1"
/>
konrad marked this conversation as resolved Outdated

Add types for emit

Add types for emit

Done

Done
<span class="list-menu-title">{{ getProjectTitle(p) }}</span>
<span class="list-menu-title">{{ getProjectTitle(project) }}</span>
</BaseButton>
<BaseButton
konrad marked this conversation as resolved Outdated

These options should either contain all dragOptions or be defined inline

These options should either contain all dragOptions or be defined inline

Moved it all inline.

Moved it all inline.
v-if="p.id > 0"
v-if="project.id > 0"
class="favorite"
:class="{'is-favorite': p.isFavorite}"
@click="projectStore.toggleProjectFavorite(p)"
:class="{'is-favorite': project.isFavorite}"
@click="projectStore.toggleProjectFavorite(project)"
>
<icon :icon="p.isFavorite ? 'star' : ['far', 'star']"/>
<icon :icon="project.isFavorite ? 'star' : ['far', 'star']"/>
</BaseButton>

Is this even necessary if we use modelValue instead of v-model for the draggable?

Is this even necessary if we use `modelValue` instead of `v-model` for the draggable?

v-model is required, using modelValue for the draggable component does not work.

`v-model` is required, using `modelValue` for the draggable component does not work.
<ProjectSettingsDropdown class="menu-list-dropdown" :project="p" v-if="p.id > 0">
<ProjectSettingsDropdown class="menu-list-dropdown" :project="project" v-if="project.id > 0">
dpschen marked this conversation as resolved Outdated

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?

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.

My bad I confused favorite projects with the 'Favorite' project.

My bad I confused favorite projects with the 'Favorite' project.
<template #trigger="{toggleOpen}">
<BaseButton class="menu-list-dropdown-trigger" @click="toggleOpen">
<icon icon="ellipsis-h" class="icon"/>
@ -66,8 +66,8 @@
<span class="list-setting-spacer" v-else></span>
</section>
<ProjectsNavigation
dpschen marked this conversation as resolved Outdated

Nice!

Nice!
v-if="!collapsedProjects[p.id]"
v-model="childProjects[p.id]"
v-if="!collapsedProjects[project.id]"

Also don't render if there are no child projects.

Also don't render if there are no child projects.

But then it won't be possible to drag a project "under" a parent to make it a child of that parent.

But then it won't be possible to drag a project "under" a parent to make it a child of that parent.
v-model="childProjects[project.id]"
:can-edit-order="true"
/>
</li>