feat: rename lists to projects #2697

Closed
konrad wants to merge 53 commits from feature/rename-lists-to-projects into main
Owner

Frontend PR for vikunja/api#1318

All cypress tests in this PR will fail until the API gets merged.

Frontend PR for https://kolaente.dev/vikunja/api/pulls/1318 All cypress tests in this PR will fail until the API gets merged.
konrad added 5 commits 2022-11-13 21:44:15 +00:00
konrad requested review from dpschen 2022-11-13 21:44:25 +00:00
dpschen was assigned by konrad 2022-11-13 21:44:31 +00:00
Member

Hi konrad!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://2697-feature-rename-lists-to-projects--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi konrad! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://2697-feature-rename-lists-to-projects--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.
konrad changed title from feature/rename-lists-to-projects to feat: rename lists to projects 2022-11-13 21:52:02 +00:00
konrad added 2 commits 2022-11-13 21:58:27 +00:00
bc24605711
Merge branch 'main' into feature/rename-lists-to-projects
# Conflicts:
#	src/components/list/ListWrapper.vue
#	src/router/index.ts
konrad added 1 commit 2022-11-13 22:06:48 +00:00
continuous-integration/drone/pr Build is failing Details
f4f324576c
fix: rename component imports
konrad force-pushed feature/rename-lists-to-projects from f4f324576c to f54aeb8e0b 2023-01-17 17:19:16 +00:00 Compare
konrad added 1 commit 2023-01-17 17:21:44 +00:00
continuous-integration/drone/pr Build is failing Details
a658201de8
fix: lint
konrad added 1 commit 2023-01-17 17:24:40 +00:00
continuous-integration/drone/pr Build is failing Details
ad169d7a72
fix: parse task text
konrad added 11 commits 2023-01-17 22:02:24 +00:00
konrad added 1 commit 2023-01-17 22:34:55 +00:00
continuous-integration/drone/pr Build is failing Details
0e6c219800
fix: cypress tests
Author
Owner

The cypress tests now work locally. The ci will fail until the API PR is merged.

The cypress tests now work locally. The ci will fail until the API PR is merged.
dpschen reviewed 2023-01-18 00:00:18 +00:00
@ -1,57 +0,0 @@
import {createFakeUserAndLogin} from '../../support/authenticateUser'

Did you use git mv for files with lots of changes? Asking because this will break a lot of history.

If not: maybe we can still do this.
I might have an idea how this could be done.

Did you use git mv for files with lots of changes? Asking because this will break a _lot_ of history. If not: maybe we can still do this. I might have an idea how this could be done.
Author
Owner

I did but this might have been broken by the rebase.

What do you have in mind?

I did but this might have been broken by the rebase. What do you have in mind?

You mean in order to restore the history again?

You mean in order to restore the history again?
Author
Owner

yes. My best idea right now is to start a new branch from main and copy the changes from this branch over. That would keep the latest history but basically remove all commits from this PR into one giant commit. Not sure what's better.

yes. My best idea right now is to start a new branch from main and copy the changes from this branch over. That would keep the latest history but basically remove all commits from this PR into one giant commit. Not sure what's better.

See my comment here: #2697 (comment)

See my comment here: https://kolaente.dev/vikunja/frontend/pulls/2697#issuecomment-42955
Member

We should make sure that:

  • list settings in local storage are converted to project settings.
  • list urls (e.g. from user bookmarks) are redirected to project urls. We should then add a warning to the user to update the bookmark.
We should make sure that: - list settings in local storage are converted to project settings. - list urls (e.g. from user bookmarks) are redirected to project urls. We should then add a warning to the user to update the bookmark.
konrad added 2 commits 2023-01-18 13:06:01 +00:00
Member

You will hate me for this question: but why did we want to rename lists to projects again? Asking because:

  • users might make lists of tasks that are not a project but simply a group.
  • since our filters are now also projects (instead of the former lists) this analogy now fails much more.
You will hate me for this question: but why did we want to rename lists to projects again? Asking because: - users might make lists of tasks that are not a project but simply a group. - since our filters are now also projects (instead of the former lists) this analogy now fails much more.
Author
Owner

but why did we want to rename lists to projects again?

Because a list now does more than just being a list. Mostly because of the different views. When I started Vikunja these did not exist so calling it a list made sense back then. I just feel like it does not make sense now.

users might make lists of tasks that are not a project but simply a group.

You mean with vikunja/api#1362?

since our filters are now also projects (instead of the former lists) this analogy now fails much more.

What do you mean?

Filters are not ideal, yes, but I think that's something for another PR.

> but why did we want to rename lists to projects again? Because a list now does more than just being a list. Mostly because of the different views. When I started Vikunja these did not exist so calling it a list made sense back then. I just feel like it does not make sense now. > users might make lists of tasks that are not a project but simply a group. You mean with https://kolaente.dev/vikunja/api/pulls/1362? > since our filters are now also projects (instead of the former lists) this analogy now fails much more. What do you mean? Filters are not ideal, yes, but I think that's something for another PR.
dpschen requested changes 2023-01-18 14:43:37 +00:00
dpschen left a comment
Member

Okay I checked all files for which there is a renaming history. The other files are basically unreviewable.

You could easily change this if you:

  1. Rebase interactive
  2. Edit the commit where the files are renamed and modified.
  3. Split that commit in two commits, where the first only renames (if the imports are broken for this commit I think that's fine) and the second makes the actual edits.
Okay I checked all files for which there is a renaming history. The other files are basically unreviewable. You could easily change this if you: 1) Rebase interactive 2) Edit the commit where the files are renamed and modified. 3) Split that commit in two commits, where the first only renames (if the imports are broken for this commit I think that's fine) and the second makes the actual edits.
@ -11,3 +11,3 @@
beforeEach(() => {
NamespaceFactory.create(1)
ListFactory.create(1)
const projects = ProjectFactory.create(1)

Why the unused variable?

Why the unused variable?
Author
Owner

Not used, removed it.

Not used, removed it.
konrad marked this conversation as resolved
scripts/run.sh Outdated
@ -21,2 +21,2 @@
sed -i "s/listen 80/listen $VIKUNJA_HTTP_PORT/g" /etc/nginx/nginx.conf
sed -i "s/listen 443/listen $VIKUNJA_HTTPS_PORT/g" /etc/nginx/nginx.conf
sed -i "s/projecten 80/projecten $VIKUNJA_HTTP_PORT/g" /etc/nginx/nginx.conf
sed -i "s/projecten 443/projecten $VIKUNJA_HTTPS_PORT/g" /etc/nginx/nginx.conf

These lines are wrong

These lines are wrong
Author
Owner

Done

Done
konrad marked this conversation as resolved
@ -53,3 +53,3 @@
<div class="namespace-title" :class="{'has-menu': n.id > 0}">
<BaseButton
@click="toggleLists(n.id)"
@click="toggleProjects(n.id)"

Feedback general to this file:

I think most renames in this file should be reverted.
Reason beeing: I see the list here as a reference to "menu list" and not to project list, even if there might be projects lists inside.

Feedback general to this file: I think most renames in this file should be reverted. Reason beeing: I see the list here as a reference to "menu list" and not to project list, even if there might be projects lists inside.
Author
Owner

Mhh yes. TBH I'd like to keep it the way it is currently since the navigation has to be reworked substantially for the move from namespaces -> projects and a lot of the effort to make it nice now would be wasted then anyway.

Mhh yes. TBH I'd like to keep it the way it is currently since the navigation has to be reworked substantially for the move from namespaces -> projects and a lot of the effort to make it nice now would be wasted then anyway.

This is exactly why I want to keep the diff minimal.
I have already a branch where I did a lot of stuff going in that direction. Would like to make the merge as easy as possible.

This is exactly why I want to keep the diff minimal. I have already a branch where I did a lot of stuff going in that direction. Would like to make the merge as easy as possible.
Author
Owner

I've reverted a few changes. Now it should only contain those where it makes sense, most notably the class names are still the old ones.

I've reverted a few changes. Now it should only contain those where it makes sense, most notably the class names are still the old ones.
konrad marked this conversation as resolved
@ -87,2 +86,3 @@
group="namespace-projects"
@start="() => drag = true"
@end="saveListPosition"
@end="saveProjectPosition"

Unsure: Doesn't describe saveListPosition this better? Since it's about the list order of the project.

Unsure: Doesn't describe saveListPosition this better? Since it's about the list order of the project.
Author
Owner

Id's say it's a project position in a list so both work. This makes it clear the function deals with projects (even though that could be inferred from the function itself)

Id's say it's a project position in a list so both work. This makes it clear the function deals with projects (even though that could be inferred from the function itself)

That makes sense. Might be another entity inside the navigation that also can save its position

That makes sense. Might be another entity inside the navigation that also can save its position
konrad marked this conversation as resolved
@ -95,3 +95,3 @@
:component-data="{
type: 'transition-group',
name: !drag ? 'flip-list' : null,
name: !drag ? 'flip-project' : null,

Might be that the flip animation is broken either way, but the class name needs to be aligned.
In the styles below I could still find a .flip-list-move which should be .flip-list-move if you still want to rename it after considering my first comment in this file.

Might be that the flip animation is broken either way, but the class name needs to be aligned. In the styles below I could still find a `.flip-list-move` which should be `.flip-list-move` __if__ you still want to rename it after considering my first comment in this file.
konrad marked this conversation as resolved
@ -105,3 +105,2 @@
<li
class="list-menu loader-container is-loading-small"
:class="{'is-loading': listUpdating[l.id]}"
class="project-menu loader-container is-loading-small"

Unsure if we should update these class names:

project-menu => By naming it after the projects we make this dependant on the project. But maybe we want to make a list of tags or whatever and could reuse the same class there.

Unsure if we should update these class names: project-menu => By naming it after the projects we make this dependant on the project. But maybe we want to make a list of tags or whatever and could reuse the same class there.
konrad marked this conversation as resolved
@ -111,2 +110,2 @@
class="list-menu-link"
:class="{'router-link-exact-active': currentList.id === l.id}"
:to="{ name: 'project.index', params: { projectId: l.id} }"
class="project-menu-link"

Same here

Same here
konrad marked this conversation as resolved
@ -79,4 +79,3 @@
icon: '<svg viewBox="0 0 24 24"><rect fill="none" rx="0" ry="0"/><path fill="currentColor" fill-rule="evenodd" clip-rule="evenodd" d="M6.5281 3.7002H3.5281C3.1981 3.7002 2.9281 3.9702 2.9281 4.3002V7.3002C2.9281 7.6302 3.1981 7.9002 3.5281 7.9002H6.5281C6.8581 7.9002 7.1281 7.6302 7.1281 7.3002V4.3002C7.1281 3.9702 6.8581 3.7002 6.5281 3.7002ZM5.9281 6.7002H4.1281V4.9002H5.9281V6.7002ZM3.5281 9.90015H6.5281C6.8581 9.90015 7.1281 10.1701 7.1281 10.5001V13.5001C7.1281 13.8301 6.8581 14.1001 6.5281 14.1001H3.5281C3.1981 14.1001 2.9281 13.8301 2.9281 13.5001V10.5001C2.9281 10.1701 3.1981 9.90015 3.5281 9.90015ZM4.1281 12.9001H5.9281V11.1001H4.1281V12.9001ZM3.5281 16.1001H6.5281C6.8581 16.1001 7.1281 16.3701 7.1281 16.7001V19.7001C7.1281 20.0301 6.8581 20.3001 6.5281 20.3001H3.5281C3.1981 20.3001 2.9281 20.0301 2.9281 19.7001V16.7001C2.9281 16.3701 3.1981 16.1001 3.5281 16.1001ZM4.1281 19.1001H5.9281V17.3001H4.1281V19.1001ZM9.72817 6.4002H20.7282C21.0582 6.4002 21.3282 6.1302 21.3282 5.8002C21.3282 5.4702 21.0582 5.2002 20.7282 5.2002H9.72817C9.39817 5.2002 9.12817 5.4702 9.12817 5.8002C9.12817 6.1302 9.39817 6.4002 9.72817 6.4002ZM9.72817 11.4001H20.7282C21.0582 11.4001 21.3282 11.6701 21.3282 12.0001C21.3282 12.3301 21.0582 12.6001 20.7282 12.6001H9.72817C9.39817 12.6001 9.12817 12.3301 9.12817 12.0001C9.12817 11.6701 9.39817 11.4001 9.72817 11.4001ZM9.72817 17.6001H20.7282C21.0582 17.6001 21.3282 17.8701 21.3282 18.2001C21.3282 18.5301 21.0582 18.8001 20.7282 18.8001H9.72817C9.39817 18.8001 9.12817 18.5301 9.12817 18.2001C9.12817 17.8701 9.39817 17.6001 9.72817 17.6001Z"/></svg>',
},
{
name: 'ordered-list',

This is wrong

This is wrong
Author
Owner

Fixed.

Fixed.
konrad marked this conversation as resolved
@ -76,3 +76,1 @@
name: 'unordered-list',
action: EasyMDE.toggleUnorderedList,
title: i18n.global.t('input.editor.unorderedList'),
name: 'unordered-project',

This is wrong

This is wrong
Author
Owner

Fixed.

Fixed.
konrad marked this conversation as resolved
@ -17,3 +17,3 @@
</message>
<dl class="shortcut-list">
<dl class="shortcut-project">

This is a list of shortcuts. Not a project.

This is a list of shortcuts. Not a project.
Author
Owner

Most definitley. Fixed.

Most definitley. Fixed.
konrad marked this conversation as resolved
@ -59,3 +59,3 @@
}
.shortcut-list {
.shortcut-project {

Same here

Same here
konrad marked this conversation as resolved
@ -18,3 +18,3 @@
{{ $t('misc.next') }}
</router-link>
<ul class="pagination-list">
<ul class="pagination-project">

This is a list of pagination items

This is a list of pagination items
Author
Owner

Fixed.

Fixed.
konrad marked this conversation as resolved
@ -9,3 +9,3 @@
<CustomTransition name="fade">
<div class="notifications-list" v-if="showNotifications" ref="popup">
<div class="notifications-project" v-if="showNotifications" ref="popup">

This is a list of notifications

This is a list of notifications
Author
Owner

Fixed.

Fixed.
konrad marked this conversation as resolved
@ -156,3 +156,3 @@
}
.notifications-list {
.notifications-project {

This is a list of notifications

This is a list of notifications
Author
Owner

Fixed.

Fixed.
konrad marked this conversation as resolved
@ -232,2 +229,2 @@
table: t('list.table.title'),
kanban: t('list.kanban.title'),
const availableViews = computed<Record<ProjectView, string>>(() => ({
project: t('project.list.title'),

The view is still called list:

list: t('project.list.title'),
The view is still called `list`: ```ts list: t('project.list.title'), ```
Author
Owner

That's true. Fixed.

That's true. Fixed.
konrad marked this conversation as resolved
@ -31,3 +31,3 @@
import {includesById} from '@/helpers/utils'
import ListUserService from '@/services/listUsers'
import ProjectUserService from '@/services/projectUsers'

Okay this renaming here helps a lot: Allthough 'project' is a verb it doesn't make as much sense as the verb 'list'. I always used to read this as 'list-the-users-service', which is not exactly what it does.

Okay this renaming here helps a lot: Allthough 'project' is a verb it doesn't make as much sense as the verb 'list'. I always used to read this as 'list-the-users-service', which is not exactly what it does.
konrad marked this conversation as resolved
@ -0,0 +7,4 @@
/>
<ColorBubble
v-if="showProjectColor && projectColor !== '' && currentProject.id !== task.projectId"

This addition shouldn't be part of this PR. This PR is so huge that we really should not do more than the renaming.

This addition shouldn't be part of this PR. This PR is so huge that we really should not do more than the renaming.
Author
Owner

What's interesting about this is I noticed this only because the test broke checking this. Not sure why.

I could either

  • add the change before the PR
  • add the change after this PR

Before might be better.

Of course, the best solution would be to fix whatever caused the test to fail but I fear that would be a change like this.

What's interesting about this is I noticed this only because the test broke checking this. Not sure why. I could either * add the change before the PR * add the change after this PR Before might be better. Of course, the best solution would be to fix whatever caused the test to fail but I fear that would be a change like this.

Do it before

Do it before
Author
Owner

PR: #2975

PR: https://kolaente.dev/vikunja/frontend/pulls/2975
konrad marked this conversation as resolved
@ -43,3 +43,3 @@
/**
* This mixin provides a base set of methods and properties to get tasks on a list.
* This mixin provides a base set of methods and properties to get tasks on a project.

Maybe we can write here now:

This mixin provides a base set of methods and properties to get tasks.

Maybe we can write here now: `This mixin provides a base set of methods and properties to get tasks.`
Author
Owner

Done

Done
konrad marked this conversation as resolved
@ -46,2 +45,3 @@
* This mixin provides a base set of methods and properties to get tasks on a project.
*/
export function useTaskList(listId, sortByDefault = SORT_BY_DEFAULT) {
export function useTaskList(projectId, sortByDefault = SORT_BY_DEFAULT) {

I use list usually as a suffix to indicate that there are multiple items. E.g. if a have a component with a for loop I will add a foobar__list to the wrapper with the v-for and a foobar__item to the individual items inside. This is the logic used for naming this. Maybe collection would be a better wording now, since after the merge of this branch 'list' will mostly refer to the layout not the data format.

I use `list` usually as a suffix to indicate that there are multiple items. E.g. if a have a component with a for loop I will add a `foobar__list` to the wrapper with the `v-for` and a `foobar__item` to the individual items inside. This is the logic used for naming this. Maybe `collection` would be a better wording now, since after the merge of this branch 'list' will mostly refer to the layout not the data format.
Author
Owner

ohh so listId here is not the id of the list but some identifier about the current "list of things"? The way it gets used in getAllTasksParams` looked like a (now) project id.

ohh so `listId` here is not the id of the list but some identifier about the current "list of things"? The way it gets used in getAllTasksParams` looked like a (now) project id.

ohh so listId here is not the id of the list but some identifier about the current "list of things"? The way it gets used in getAllTasksParams` looked like a (now) project id.

Tbh I'm unsure about this. I have also adjusted this massively and don't rembember the original function as it is here very well.

Can we keep this for now as useTaskList? I could still change the name later (maybe just useProject?).

> ohh so listId here is not the id of the list but some identifier about the current "list of things"? The way it gets used in getAllTasksParams` looked like a (now) project id. Tbh I'm unsure about this. I have also adjusted this massively and don't rembember the original function as it is here very well. Can we keep this for now as useTaskList? I could still change the name later (maybe just `useProject`?).
Author
Owner

so leaving it as it is now? The name of the function did not change, only the parameter.

so leaving it as it is now? The name of the function did not change, only the parameter.

Yes, that's what I meant :)

Yes, that's what I meant :)
konrad marked this conversation as resolved
@ -99,3 +99,3 @@
// Only listen for query path changes
// Only projecten for query path changes

listen

`listen`
Author
Owner

Done.

Done.
konrad marked this conversation as resolved
@ -423,3 +423,3 @@
"import": "Import your data into Vikunja",
"description": "Click on the logo of one of the third-party services below to get started.",
"descriptionDo": "Vikunja will import all lists, tasks, notes, reminders and files you have access to.",
"descriptionDo": "Vikunja will import all projects, tasks, notes, reminders and files you have access to.",

Probably this should stay lists, because in other tools this is not always called projects.

Probably this should stay lists, because in other tools this is not always called projects.
Author
Owner

yes! Done.

yes! Done.
konrad marked this conversation as resolved
@ -531,2 +531,2 @@
"unorderedList": "Unordered List",
"orderedList": "Ordered List",
"unorderedProject": "Unordered Project",
"orderedProject": "Ordered Project",

Wrong!

Wrong!
Author
Owner

whooops

Fixed.

whooops Fixed.
konrad marked this conversation as resolved
@ -903,1 +901,3 @@
"switchToListView": "Switch to list view",
"project": {
"title": "Project Views",
"switchToProjectView": "Switch to project view",

The view stays a list.

The view stays a list.
Author
Owner

Fixed!

Fixed!
konrad marked this conversation as resolved
@ -24,3 +23,1 @@
import LinkShareAuthComponent from '@/views/sharing/LinkSharingAuth.vue'
const ListNamespaces = () => import('@/views/namespaces/ListNamespaces.vue')
const TaskDetailView = () => import('@/views/tasks/TaskDetailView.vue')
import UpcomingTasksComponent from '../views/tasks/ShowTasks.vue'

Why the change to .. instead of @?

Why the change to `..` instead of `@`?
Author
Owner

Not intended, probably caused by the rebase.

Not intended, probably caused by the rebase.

Can we keep the @ to keep it aligned?

Can we keep the `@` to keep it aligned?
Author
Owner

Sure, changed all imports so they do that.

Sure, changed all imports so they do that.
konrad marked this conversation as resolved
@ -358,2 +357,3 @@
name: 'project.index',
redirect(to) {
// Redirect the user to list view by default
// Redirect the user to project view by default

This redirects indead to the list view!

This redirects indead to the `list` view!
konrad marked this conversation as resolved
@ -361,2 +361,2 @@
const savedListView = getListView(to.params.listId)
console.debug('Replaced list view with', savedListView)
const savedProjectView = getProjectView(to.params.projectId)
console.debug('Replaced project view with', savedProjectView)

Should be:

console.debug('Replaced list view with', savedProjectView)
Should be: ```ts console.debug('Replaced list view with', savedProjectView) ```
Author
Owner

I'd argue this is a project view here. It can be any of the four views (or at least that's what I meant when I originally added the log).

I'd argue this is a project view here. It can be any of the four views (or at least that's what I meant when I originally added the log).

I thought that it was refering to the default view which is the 'list'. That's why it's replacing it. at all. If there is no saved view nothing will be replaced and the default stays list.

I thought that it was refering to the default view which is the 'list'. That's why it's replacing it. at all. If there is no saved view nothing will be replaced and the default stays list.
Author
Owner

okay that makes sense.

okay that makes sense.
Author
Owner

Changed!

Changed!
konrad marked this conversation as resolved
@ -0,0 +4,4 @@
export default class ProjectUserService extends AbstractService {
constructor() {
super({
getAll: '/projects/{projectId}/projectusers',

Can we make the endpoint here only /projects/{projectId}/users instead?

Can we make the endpoint here only `/projects/{projectId}/users` instead?
Author
Owner

What should we use to get the users a project is currently shared with then? Because that's /projects/{projectId}/users right now.

The difference between the two is /projectusers also returns users who have access to a project because they're part of a team.

What should we use to get the users a project is currently shared with then? Because that's `/projects/{projectId}/users` right now. The difference between the two is `/projectusers` also returns users who have access to a project because they're part of a team.

The term members comes to my mind, but I guess this is all not something that should be changed here. If the project prefix could be deleted I would have found it fine to include it in this PR, but with your explanation it doesn't make much sense anymore.

The term `members` comes to my mind, but I guess this is all not something that should be changed here. If the `project` prefix could be deleted I would have found it fine to include it in this PR, but with your explanation it doesn't make much sense anymore.
dpschen marked this conversation as resolved
@ -65,1 +63,3 @@
if (l.subscription === null || l.subscription.entity !== 'list') {
// Check for each project in that namespace if it has a subscription and set it if not
n.projects.forEach(l => {
if (l.subscription === null || l.subscription.entity !== 'project') {

Just a question here: How was this info saved in the DB? Will the migration take care the the entitiy now can be called project instead of list?

Just a question here: How was this info saved in the DB? Will the migration take care the the entitiy now can be called `project` instead of `list`?
Author
Owner

The migration will Take care of that. The DB only saves integers.

The migration will Take care of that. The DB only saves integers.

Cool! :)

Cool! :)
konrad marked this conversation as resolved
@ -344,2 +343,3 @@
let foundProjectId = null
// Uses the following ways to get the list id of the new task:
console.log('find', {projectName, projectId})

Here is some leftover

Here is some leftover
Author
Owner

whoops, removed.

whoops, removed.
konrad marked this conversation as resolved
@ -347,3 +349,1 @@
if (listName !== null) {
const list = listStore.findListByExactname(listName)
foundListId = list === null ? null : list.id
if (typeof projectName !== 'undefined' && projectName !== null) {

Is this a changed new condition or are we reverting here something that should only check for !== null?

Is this a changed new condition or are we reverting here something that should only check for `!== null`?
Author
Owner

I discovered how it failed when projectName was undefined and it should behave the same as the if. It's a changed new condition.

I discovered how it failed when `projectName` was undefined and it should behave the same as the if. It's a changed new condition.
konrad marked this conversation as resolved
@ -37,0 +37,4 @@
$projects-per-row: 5;
$project-height: 150px;
$project-spacing: 1rem;

Where are these coming from?

Where are these coming from?
Author
Owner

rebase, mostly. Removed them since you already moved them elsewhere.

rebase, mostly. Removed them since you already moved them elsewhere.
konrad marked this conversation as resolved
@ -9,3 +9,3 @@
background: var(--grey-200);
// FIXME: only used in ListLabels.vue
// FIXME: only used in ProjectLabels.vue

I think you didn't rename that, which would make sense, since it "lists the labels"

I think you didn't rename that, which would make sense, since it "lists the labels"
Author
Owner

yes 👍

yes 👍
konrad marked this conversation as resolved
@ -0,0 +6,4 @@
$filter-container-top-default: -59px;
$filter-container-top-link-share-gantt: -133px;
$filter-container-top-link-share-project: -47px;

This is for the "list-view" so it should stay $filter-container-top-link-share-list

This is for the "list-view" so it should stay `$filter-container-top-link-share-list`
Author
Owner

Correct. Fixed!

Correct. Fixed!
konrad marked this conversation as resolved
@ -0,0 +97,4 @@
margin-top: calc(#{$filter-container-top-link-share-gantt} - 5rem);
}
.link-share-container .project-view .filter-container {

Should stay list-view

Should stay `list-view`
Author
Owner

Done.

Done.
konrad marked this conversation as resolved
@ -0,0 +98,4 @@
}
.link-share-container .project-view .filter-container {
margin-top: $filter-container-top-link-share-project - 10px;

This is for the "list-view" so it should stay $filter-container-top-link-share-list

This is for the "list-view" so it should stay `$filter-container-top-link-share-list`
Author
Owner

Done

Done
konrad marked this conversation as resolved
@ -0,0 +102,4 @@
}
.link-share-container.project\.table-view,
.link-share-container.project\.project-view {

Should stay .list-view.

Should stay `.list-view`.
Author
Owner

Done.

Done.
konrad marked this conversation as resolved
@ -20,2 +20,2 @@
&.list\.gantt-view,
&.list\.kanban-view {
&.project\.gantt-view,
&.project\.kanban-view {

Unsure what list is meant with this.

Unsure what list is meant with this.
Author
Owner

I think the literal list (the one now called "project").

I think the literal list (the one now called "project").
konrad marked this conversation as resolved
@ -32,3 +32,3 @@
.link-share-container:not(.has-background) {
.list-view {
.project-view {

Should stay .list-view

Should stay `.list-view`
konrad marked this conversation as resolved
src/sw.ts Outdated
@ -25,3 +25,3 @@
)
// This code listens for the user's confirmation to update the app.
// This code projectens for the user's confirmation to update the app.

listens

`listens`
konrad marked this conversation as resolved
@ -12,3 +12,3 @@
icon: IconProp
mask?: IconLookup
listItem?: boolean
projectItem?: boolean

Wrong!

Wrong!
konrad marked this conversation as resolved
@ -15,3 +15,3 @@
</message>
<add-task
@taskAdded="updateTaskList"
@taskAdded="updateTaskProject"

This should either:

  • stay the same (since it could refer to the list layout that we show here) or
  • be updated to updateProject.
This should either: - stay the same (since it could refer to the list layout that we show here) or - be updated to `updateProject`.
Author
Owner

Yes. I've updated it.

Yes. I've updated it.
konrad marked this conversation as resolved
@ -99,3 +99,3 @@
const deletionScheduledAt = computed(() => parseDateOrNull(authStore.info?.deletionScheduledAt))
// This is to reload the tasks list after adding a new task through the global task add.
// This is to reload the tasks project after adding a new task through the global task add.

// This is to reload the project after adding a new task through the global task add.

(remove tasks)

`// This is to reload the project after adding a new task through the global task add.` (remove `tasks`)
konrad marked this conversation as resolved
@ -103,3 +103,3 @@
const showTasksKey = ref(0)
function updateTaskList() {
function updateTaskProject() {

See above.

See above.
konrad marked this conversation as resolved
@ -55,1 +52,3 @@
{{ $t('namespace.createList') }}
<p v-if="n.projects.length === 0" class="has-text-centered has-text-grey mt-4 is-italic">
{{ $t('namespace.noProjects') }}
<BaseButton :to="{name: 'projects.create', params: {namespaceId: n.id}}">

Didn't check, but shouldn't this be name: 'project.create' since list was also singular here.

Didn't check, but shouldn't this be `name: 'project.create'` since list was also singular here.
Author
Owner

Yes, it should! Nice catch.

Fixed.

Yes, it should! Nice catch. Fixed.
konrad marked this conversation as resolved
@ -0,0 +1,324 @@
<template>
<ProjectWrapper class="project-project" :project-id="projectId" viewName="project">
class="project-list"
```html class="project-list" ```
konrad marked this conversation as resolved
@ -0,0 +50,4 @@
<template #default>
<div
:class="{ 'is-loading': loading }"
class="loader-container is-max-width-desktop project-view"

This should stay list-view

This should stay `list-view`
konrad marked this conversation as resolved
@ -0,0 +55,4 @@
<card :padding="false" :has-content="false" class="has-overflow">
<add-task
v-if="!project.isArchived && canWrite"
class="project-view__add-task"

This should stay list-view__add-task

This should stay `list-view__add-task`
konrad marked this conversation as resolved
@ -0,0 +116,4 @@
</template>
<script lang="ts">
export default { name: 'Project' }

This should stay the List view.

This should stay the `List` view.
konrad marked this conversation as resolved
@ -0,0 +313,4 @@
}
}
.project-view__add-task {

This should stay list-view

This should stay `list-view`
konrad marked this conversation as resolved
@ -0,0 +36,4 @@
<BaseButton class="unsplash-credit__link" href="https://unsplash.com">{{ $t('project.background.poweredByUnsplash') }}</BaseButton>
</p>
<ul class="image-search__result-project">

This is a result-list

This is a `result-list`
konrad marked this conversation as resolved
@ -0,0 +236,4 @@
color: var(--grey-800);
}
.image-search__result-project {

This is a result-list

This is a `result-list`
konrad marked this conversation as resolved
konrad added 1 commit 2023-01-18 14:46:21 +00:00
continuous-integration/drone/pr Build is failing Details
1d2d4a2363
fix: wrong projecten rename from lists
konrad added 2 commits 2023-01-18 14:50:46 +00:00
konrad added 1 commit 2023-01-18 14:51:46 +00:00
continuous-integration/drone/pr Build is failing Details
68cf0e30ce
fix: shortcut list class name
Member

Because a list now does more than just being a list. Mostly because of the different views. When I started Vikunja these did not exist so calling it a list made sense back then. I just feel like it does not make sense now.

Would have proably been cheaper to use typescript consequently and name the list view list layout, since 'view' is almost a synonym for pages. Then again 'layout' has its own semantics in Nuxt… :D

users might make lists of tasks that are not a project but simply a group.

No has nothing to do with that branch. I mean something like 'Inbox'. Or 'Foobar' (where I put stuff that doesn't belong anywhere else).
Even If I name something 'Longterm' it isn't really a project.

since our filters are now also projects (instead of the former lists) this analogy now fails much more.

This is more about how we use filters internally. What I mean is:

  • A filter is a kind of a list can make sense in our context.
  • A filter is a kind of a project will probably not be true in a lot of cases. (e.g. I filter for tasks that are due next week, that is not what you would understand as a project).

So what I mean is that the analogy that we had until now "A filter is a kind of a list" doesn't work anymore.

Filters are not ideal, yes, but I think that's something for another PR.

True, but not what I meant :D

> Because a list now does more than just being a list. Mostly because of the different views. When I started Vikunja these did not exist so calling it a list made sense back then. I just feel like it does not make sense now. Would have proably been cheaper to use typescript consequently and name the list view list layout, since 'view' is almost a synonym for pages. Then again 'layout' has its own semantics in Nuxt… :D > users might make lists of tasks that are not a project but simply a group. No has nothing to do with that branch. I mean something like 'Inbox'. Or 'Foobar' (where I put stuff that doesn't belong anywhere else). Even If I name something 'Longterm' it isn't really a project. > since our filters are now also projects (instead of the former lists) this analogy now fails much more. This is more about how we use filters internally. What I mean is: - A filter is a kind of a list can make sense in our context. - A filter is a kind of a project will probably not be true in a lot of cases. (e.g. I filter for tasks that are due next week, that is not what you would understand as a project). So what I mean is that the analogy that we had until now "A filter is a kind of a list" doesn't work anymore. > Filters are not ideal, yes, but I think that's something for another PR. True, but not what I meant :D
konrad added 16 commits 2023-01-18 15:18:53 +00:00
konrad added 3 commits 2023-01-18 15:21:38 +00:00
konrad added 1 commit 2023-01-18 15:36:29 +00:00
continuous-integration/drone/pr Build is failing Details
62262e5397
fix: animation class list name
Author
Owner

Would have proably been cheaper to use typescript consequently and name the list view list layout, since 'view' is almost a synonym for pages. Then again 'layout' has its own semantics in Nuxt… :D

Yeah well if only I'd known that when I started Vikunja 4 years ago :D

No has nothing to do with that branch. I mean something like 'Inbox'. Or 'Foobar' (where I put stuff that doesn't belong anywhere else).
Even If I name something 'Longterm' it isn't really a project.

Not sure if I understand you correctly, but there will be a default Project "Inbox" instead of a new empty namespace once that PR other is merged.

I think it's fine to have a project like that where you put stuff that does not belong anywhere else, even if it does not make sense to call that a project. The rename is a compromise between lists being more than a simple list and use-cases like that.

So what I mean is that the analogy that we had until now "A filter is a kind of a list" doesn't work anymore.

That is true. This is something that might change with the move from namespaces -> projects, not sure about that yet.

> Would have proably been cheaper to use typescript consequently and name the list view list layout, since 'view' is almost a synonym for pages. Then again 'layout' has its own semantics in Nuxt… :D Yeah well if only I'd known that when I started Vikunja 4 years ago :D > No has nothing to do with that branch. I mean something like 'Inbox'. Or 'Foobar' (where I put stuff that doesn't belong anywhere else). Even If I name something 'Longterm' it isn't really a project. Not sure if I understand you correctly, but there will be a default Project "Inbox" instead of a new empty namespace once that PR other is merged. I think it's fine to have a project like that where you put stuff that does not belong anywhere else, even if it does not make sense to call that a project. The rename is a compromise between lists being more than a simple list and use-cases like that. > So what I mean is that the analogy that we had until now "A filter is a kind of a list" doesn't work anymore. That is true. This is something that might change with the move from namespaces -> projects, not sure about that yet.
konrad added 1 commit 2023-01-18 15:52:13 +00:00
continuous-integration/drone/pr Build is failing Details
31eee4c2f1
fix: change debug log
konrad added 1 commit 2023-01-18 16:02:02 +00:00
continuous-integration/drone/pr Build is failing Details
3f82dbe0df
fix: revert a few navigation changes
Member

Yeah well if only I'd known that when I started Vikunja 4 years ago :D

:D

Not sure if I understand you correctly, but there will be a default Project "Inbox" instead of a new empty namespace once that PR other is merged.

Oh that's cool.

I think it's fine to have a project like that where you put stuff that does not belong anywhere else, even if it does not make sense to call that a project. The rename is a compromise between lists being more than a simple list and use-cases like that.

Absolutely, I just wanted to point out the consequences. It similar to that it doesn't make sense to put movies in folders.

That is true. This is something that might change with the move from namespaces -> projects, not sure about that yet.

All good. As said, mostly wanted to understand and point out 👍

> Yeah well if only I'd known that when I started Vikunja 4 years ago :D :D > Not sure if I understand you correctly, but there will be a default Project "Inbox" instead of a new empty namespace once that PR other is merged. Oh that's cool. > I think it's fine to have a project like that where you put stuff that does not belong anywhere else, even if it does not make sense to call that a project. The rename is a compromise between lists being more than a simple list and use-cases like that. Absolutely, I just wanted to point out the consequences. It similar to that it doesn't make sense to put movies in folders. > That is true. This is something that might change with the move from namespaces -> projects, not sure about that yet. All good. As said, mostly wanted to understand and point out 👍
konrad added 1 commit 2023-01-18 16:08:01 +00:00
continuous-integration/drone/pr Build is failing Details
da75638b12
fix: use @ for imports of all views
Author
Owner

@dpschen Anything else missing? From my pov this is all good now.

@dpschen Anything else missing? From my pov this is all good now.
dpschen reviewed 2023-01-18 21:36:34 +00:00
@ -75,3 +75,3 @@
<table
class="table has-actions is-striped is-hoverable is-fullwidth link-share-list"
class="table has-actions is-striped is-hoverable is-fullwidth link-share-project"

This is the list of shared links

This is the list of shared links

Althought it probably should be a table.

Althought it probably should be a table.
Author
Owner

The class isn't even used anywhere. I've removed it.

The class isn't even used anywhere. I've removed it.
konrad marked this conversation as resolved
konrad added 1 commit 2023-01-18 21:43:16 +00:00
continuous-integration/drone/pr Build is failing Details
d4eda3784f
fix: rename link share list class
dpschen reviewed 2023-01-18 21:47:24 +00:00
@ -23,3 +23,3 @@
import UpcomingTasksComponent from '@/views/tasks/ShowTasks.vue'
import LinkShareAuthComponent from '@/views/sharing/LinkSharingAuth.vue'
const ListNamespaces = () => import('@/views/namespaces/ListNamespaces.vue')
import ListNamespaces from '@/views/namespaces/ListNamespaces.vue'

Now these are not a dynamic imports anymore.

Now these are not a dynamic imports anymore.
Author
Owner

Fixed.

Fixed.
konrad marked this conversation as resolved
konrad added 1 commit 2023-01-18 21:48:32 +00:00
continuous-integration/drone/pr Build is failing Details
f60e4dd89d
fix: remove unused css class
konrad added 1 commit 2023-01-18 21:52:18 +00:00
continuous-integration/drone/pr Build is failing Details
163885af81
fix: dynamically import project components again
Member

Can we close this in favor of #2978

Can we close this in favor of https://kolaente.dev/vikunja/frontend/pulls/2978
Author
Owner

Yes.

Yes.
konrad closed this pull request 2023-02-10 12:48:31 +00:00
This repo is archived. You cannot comment on pull requests.
No description provided.