vikunja/frontend
Archived
6
39
Fork 48
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 +19 -30
2 changed files with 19 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 65522a57f1 - Show all commits

28
src/components/home/ProjectsNavigationWrapper.vue
View File

@ -1,28 +0,0 @@
<template>
<nav class="menu" v-if="favoriteProjects">
<ProjectsNavigation :model-value="favoriteProjects" :can-edit-order="false"/>
</nav>
<nav class="menu">
<ProjectsNavigation :model-value="projects" :can-edit-order="true"/>
</nav>
</template>
<script setup lang="ts">
import {computed} from 'vue'
import {useProjectStore} from '@/stores/projects'
import ProjectsNavigation from '@/components/home/ProjectsNavigation.vue'
const projectStore = useProjectStore()
const projects = computed(() => projectStore.notArchivedRootProjects
.sort((a, b) => a.position - b.position))
const favoriteProjects = computed(() => projectStore.favoriteProjects
.sort((a, b) => a.position - b.position))
</script>
<style lang="scss" scoped>
.menu + .menu{
padding-top: math.div($navbar-padding, 2);
}
</style>

21
src/components/home/navigation.vue
View File

@ -49,7 +49,15 @@
</nav>
<Loading variant="small" v-if="projectsLoading"/>
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-04-03 10:06:39 +00:00
Outdated
Review
Copy Link

Move v-if to front

Move `v-if` to front
✅ 1
konrad commented 2023-04-10 18:59:51 +00:00
Outdated
Review
Copy Link

Done

Done
<ProjectsNavigationWrapper v-else/>
<template v-else>
<nav class="menu" v-if="favoriteProjects">
<ProjectsNavigation :model-value="favoriteProjects" :can-edit-order="false"/>
</nav>
<nav class="menu">
<ProjectsNavigation :model-value="projects" :can-edit-order="true"/>
</nav>
</template>
<PoweredByLink/>
</aside>
@ -64,12 +72,17 @@ import Loading from '@/components/misc/loading.vue'
import {useBaseStore} from '@/stores/base'
import {useProjectStore} from '@/stores/projects'
import ProjectsNavigationWrapper from '@/components/home/ProjectsNavigationWrapper.vue'
import ProjectsNavigation from '@/components/home/ProjectsNavigation.vue'
const baseStore = useBaseStore()
dpschen commented 2023-03-28 13:17:48 +00:00
Outdated
Review
Copy Link

Can we put this component inside a <Suspense>? Then we can use await methods directly and without onBeforeMount hook.

Can we put this component inside a `<Suspense>`? Then we can use `await` methods directly and without `onBeforeMount ` hook.
konrad commented 2023-03-28 14:20:04 +00:00
Outdated
Review
Copy Link

I've now moved the project navigation into a separate wrapper component so that we can show a loading spinner while projects are loading and still show the other navigation links (overview, labels, etc).

I've now moved the project navigation into a separate wrapper component so that we can show a loading spinner while projects are loading and still show the other navigation links (overview, labels, etc).
dpschen commented 2023-04-01 12:26:13 +00:00
Outdated
Review
Copy Link

Ok. Will check

Ok. Will check
const projectStore = useProjectStore()
const menuActive = computed(() => baseStore.menuActive)
const projectsLoading = computed(() => projectStore.isLoading)
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-04-03 10:07:42 +00:00
Outdated
Review
Copy Link

Remove both computed above and use store + property directly instead.

Remove both computed above and use store + property directly instead.
✅ 1
konrad commented 2023-04-10 18:59:37 +00:00
Outdated
Review
Copy Link

Done

Done
const projects = computed(() => projectStore.notArchivedRootProjects
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-03-28 13:18:48 +00:00
Outdated
Review
Copy Link

This seems like something that the store should export instead as a computed.

This seems like something that the store should export instead as a computed.
✅ 1
konrad commented 2023-03-28 13:34:19 +00:00
Outdated
Review
Copy Link

That makes sense.

That makes sense.
konrad commented 2023-03-28 13:36:41 +00:00
Outdated
Review
Copy Link

Changed it.

Changed it.
.sort((a, b) => a.position - b.position))
const favoriteProjects = computed(() => projectStore.favoriteProjects
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-03-28 13:24:30 +00:00
Outdated
Review
Copy Link

Simplify to

.sort((a, b) => a.position - b.position)
Simplify to ```ts .sort((a, b) => a.position - b.position) ```
✅ 1
konrad commented 2023-03-28 13:37:49 +00:00
Outdated
Review
Copy Link

Done.

Done.
.sort((a, b) => a.position - b.position))
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-03-28 13:19:22 +00:00
Outdated
Review
Copy Link

This computed as well.

This computed as well.
👍 1 ✅ 1
</script>
<style lang="scss" scoped>
dpschen commented 2023-04-12 14:58:49 +00:00
Outdated
Review
Copy Link

This sorts the returned computed of the store
=> sort already in store OR create copy (worse performance)

Error in vue developer tools:

[Vue warn] Set operation on key "0" failed: target is readonly. [...]

This sorts the returned computed of the store => sort already in store OR create copy (worse performance) Error in vue developer tools: > [Vue warn] Set operation on key "0" failed: target is readonly. [...]
konrad commented 2023-04-14 15:21:55 +00:00
Outdated
Review
Copy Link

Now sorting in store, that seems to work (or at least there are no errors now)

Now sorting in store, that seems to work (or at least there are no errors now)
@ -126,4 +139,8 @@ const projectsLoading = computed(() => projectStore.isLoading)
}
}
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-04-01 16:38:47 +00:00
Outdated
Review
Copy Link

This changes styles inside the component. If the styles need to be adjusted the component should be changed instead.

This changes styles inside the component. If the styles need to be adjusted the component should be changed instead.
✅ 1
konrad commented 2023-04-01 19:54:41 +00:00
Outdated
Review
Copy Link

The problem is the component is used in multiple places where this would need different sizes. The way to do this would probably be variants with a prop?

The problem is the component is used in multiple places where this would need different sizes. The way to do this would probably be variants with a prop?
dpschen commented 2023-04-01 21:57:22 +00:00
Outdated
Review
Copy Link

Correct! The question to ask is also: do we even need so many different sizes or shouldn't they be more unied. For now I guess it's enough to answer this quesion for this usecase here.

Correct! The question to ask is also: do we even need so many different sizes or shouldn't they be more unied. For now I guess it's enough to answer this quesion for this usecase here.
konrad commented 2023-04-02 12:45:26 +00:00
Outdated
Review
Copy Link

I've now added this as a variant to the component. In doing this I discovered there are more styles and uses of that loader which we should refactor at some point. I would consider that out of scope for this PR though.

I've now added this as a variant to the component. In doing this I discovered there are more styles and uses of that loader which we should refactor at some point. I would consider that out of scope for this PR though.
}
.menu + .menu{
dpschen marked this conversation as resolved Outdated
dpschen commented 2023-04-03 10:08:06 +00:00
Outdated
Review
Copy Link

Missing space

Missing space
✅ 1
konrad commented 2023-04-10 18:59:21 +00:00
Outdated
Review
Copy Link

Done

Done
padding-top: math.div($navbar-padding, 2);
}
</style>