vikunja/frontend
Archived
6
39
Fork 49
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

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
Conversation 16 Commits 123 Files Changed 85 +102 -75
2 changed files with 102 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3db4e011d4 - Show all commits

83
src/components/home/ProjectsNavigation.vue
View File

@ -24,47 +24,12 @@
:class="{'is-loading': projectUpdating[project.id]}"
:data-project-id="project.id"
>
<section>
<BaseButton
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[project.id] }"/>
</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">
<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>
<ProjectsNavigationItem
dpschen commented 2023-04-01 16:10:21 +00:00
Outdated
Review
Copy Link

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!)
dpschen commented 2023-04-01 16:12:15 +00:00
Outdated
Review
Copy Link

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.
konrad commented 2023-04-01 19:47:57 +00:00
Outdated
Review
Copy Link

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
konrad commented 2023-04-01 19:48:18 +00:00
Outdated
Review
Copy Link

Shouldn't a nav hold multiple navigation items?

Shouldn't a `nav` hold multiple navigation items?
dpschen commented 2023-04-02 17:04:33 +00:00
Outdated
Review
Copy Link

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) }) ```
dpschen commented 2023-04-02 17:06:36 +00:00
Outdated
Review
Copy Link

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`
konrad commented 2023-04-02 17:53:59 +00:00
Outdated
Review
Copy Link

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.
: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]"
@ -76,19 +41,15 @@
</template>
<script lang="ts" setup>
import {ref, computed, watch} from 'vue'
import {ref, watch} from 'vue'
import draggable from 'zhyswan-vuedraggable'
import type {SortableEvent} from 'sortablejs'
import BaseButton from '@/components/base/BaseButton.vue'
import ProjectSettingsDropdown from '@/components/project/project-settings-dropdown.vue'
import ProjectsNavigationItem from '@/components/home/ProjectsNavigationItem.vue'
konrad marked this conversation as resolved Outdated
dpschen commented 2023-04-03 09:59:40 +00:00
Outdated
Review
Copy Link

Add types for emit

Add types for emit
konrad commented 2023-04-10 19:01:49 +00:00
Outdated
Review
Copy Link

Done

Done
import {calculateItemPosition} from '@/helpers/calculateItemPosition'
import {getProjectTitle} from '@/helpers/getProjectTitle'
import type {IProject} from '@/modelTypes/IProject'
konrad marked this conversation as resolved Outdated
dpschen commented 2023-04-03 10:03:14 +00:00
Outdated
Review
Copy Link

These options should either contain all dragOptions or be defined inline

These options should either contain all dragOptions or be defined inline
konrad commented 2023-04-10 19:01:34 +00:00
Outdated
Review
Copy Link

Moved it all inline.

Moved it all inline.
import ColorBubble from '@/components/misc/colorBubble.vue'
import {useBaseStore} from '@/stores/base'
import {useProjectStore} from '@/stores/projects'
const props = defineProps<{
@ -103,9 +64,7 @@ const dragOptions = {
ghostClass: 'ghost',
}
const baseStore = useBaseStore()
const projectStore = useProjectStore()
const currentProject = computed(() => baseStore.currentProject)
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-03-28 13:10:01 +00:00
Outdated
Review
Copy Link

Nice!

Nice!
🎉 1
// Vue draggable will modify the projects list as it changes their position which will not work on a prop.
dpschen commented 2023-04-01 13:02:26 +00:00
Outdated
Review
Copy Link

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

Also don't render if there are no child projects.
konrad commented 2023-04-01 14:52:01 +00:00
Outdated
Review
Copy Link

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.
// Hence, we'll clone the prop and work on the clone.
@ -170,29 +129,3 @@ async function saveProjectPosition(e: SortableEvent) {
}
}
</script>
<style lang="scss" scoped>
.list-setting-spacer {
width: 5rem;
flex-shrink: 0;
}
.project-is-collapsed {
transform: rotate(-90deg);
}
.favorite {
transition: opacity $transition, color $transition;
opacity: 0;
&:hover,
&.is-favorite {
opacity: 1;
color: var(--warning);
}
}
.list-menu:hover > section > .favorite {
opacity: 1;
}
</style>

94
src/components/home/ProjectsNavigationItem.vue Normal file
View File

@ -0,0 +1,94 @@
<template>
<section>
<BaseButton
v-if="canCollapse"
@click="emit('collapse')"
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-04-02 23:03:51 +00:00
Outdated
Review
Copy Link

Set from outside, since this id is related to the sorting.

Set from outside, since this id is related to the sorting.
✅ 1
konrad commented 2023-04-10 19:01:13 +00:00
Outdated
Review
Copy Link

Done

Done
class="collapse-project-button"
>
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-04-02 23:00:20 +00:00
Outdated
Review
Copy Link

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.

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.
✅ 1
konrad commented 2023-04-10 19:00:58 +00:00
Outdated
Review
Copy Link

Done

Done
<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}"
>
<span class="icon menu-item-icon handle">
<icon icon="grip-lines"/>
</span>
<ColorBubble
v-if="project.hexColor !== ''"
:color="project.hexColor"
dpschen commented 2023-04-02 23:00:55 +00:00
Outdated
Review
Copy Link

Fix indention

Fix indention
dpschen commented 2023-04-02 23:02:08 +00:00
Outdated
Review
Copy Link

Pass 'handle class name from parent => separate concerns / source of truth

Pass 'handle class name from parent => separate concerns / source of truth
konrad commented 2023-04-10 18:50:36 +00:00
Outdated
Review
Copy Link

But the indention is correct?

But the indention is correct?
👍 1
konrad commented 2023-04-10 18:51:25 +00:00
Outdated
Review
Copy Link

Pass 'handle class name from parent => separate concerns / source of truth

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 2023-04-11 14:31:35 +00:00
Outdated
Review
Copy Link

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.

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.
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>
</template>
<script setup lang="ts">
import {computed} from 'vue'
import {useProjectStore} from '@/stores/projects'
import {useBaseStore} from '@/stores/base'
dpschen commented 2023-04-02 22:58:31 +00:00
Outdated
Review
Copy Link

Use Expandable component for this.

Use `Expandable` component for this.
konrad commented 2023-04-10 19:00:38 +00:00
Outdated
Review
Copy Link

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 2023-04-11 14:26:34 +00:00
Outdated
Review
Copy Link

I created an example how to use this in 51e29af010. 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).

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`).
import type {IProject} from '@/modelTypes/IProject'
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'
defineProps<{
project: IProject,
isCollapsed: boolean,
canCollapse: boolean,
}>()
const emit = defineEmits(['collapse'])
const projectStore = useProjectStore()
const baseStore = useBaseStore()
const currentProject = computed(() => baseStore.currentProject)
</script>
<style lang="scss" scoped>
dpschen commented 2023-04-11 15:09:37 +00:00
Outdated
Review
Copy Link

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?

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 2023-04-12 08:52:06 +00:00
Outdated
Review
Copy Link

Added.

Could it be that ts doesn't display errors in the editor for you?

Looks like it 🤔

Added. > Could it be that ts doesn't display errors in the editor for you? Looks like it 🤔
.list-setting-spacer {
width: 5rem;
flex-shrink: 0;
}
.project-is-collapsed {
transform: rotate(-90deg);
}
.favorite {
transition: opacity $transition, color $transition;
opacity: 0;
&:hover,
&.is-favorite {
opacity: 1;
color: var(--warning);
}
}
.list-menu:hover > section > .favorite {
opacity: 1;
}
</style>
dpschen commented 2023-04-12 14:56:16 +00:00
Outdated
Review
Copy Link

Simplify:

const canNestDeeper = computed(() => props.level >= 2 && window.PROJECT_INFINITE_NESTING_ENABLED)
Simplify: ```ts const canNestDeeper = computed(() => props.level >= 2 && window.PROJECT_INFINITE_NESTING_ENABLED) ```
konrad commented 2023-04-14 15:19:47 +00:00
Outdated
Review
Copy Link

But that would return false for the first two levels?

But that would return `false` for the first two levels?