Add default view setting #2306

Merged
konrad merged 11 commits from Elscrux/vikunja:feature/default-view-setting into main 2024-06-02 08:15:56 +00:00
6 changed files with 53 additions and 6 deletions

View File

@ -95,6 +95,7 @@
"weekStartMonday": "Monday",
"language": "Language",
"defaultProject": "Default Project",
"defaultView": "Default View",
"timezone": "Time Zone",
"overdueTasksRemindersTime": "Overdue tasks reminder email time",
"filterUsedOnOverview": "Saved filter used on the overview page"
@ -314,6 +315,9 @@
"delete": "Delete"
}
},
"first": {
"title": "First View"
},
"list": {
"title": "List",
"add": "Add",

View File

@ -9,6 +9,12 @@ export const PROJECT_VIEW_KINDS = {
} as const
export type ProjectViewKind = typeof PROJECT_VIEW_KINDS[keyof typeof PROJECT_VIEW_KINDS]
export const DEFAULT_PROJECT_VIEW_SETTINGS = {
FIRST: 'first',
...PROJECT_VIEW_KINDS,
} as const
export type DefaultProjectViewKind = typeof DEFAULT_PROJECT_VIEW_SETTINGS[keyof typeof DEFAULT_PROJECT_VIEW_SETTINGS]
export const PROJECT_VIEW_BUCKET_CONFIGURATION_MODES = ['none', 'manual', 'filter']
export type ProjectViewBucketConfigurationMode = typeof PROJECT_VIEW_BUCKET_CONFIGURATION_MODES[number]

View File

@ -4,12 +4,14 @@ import type {IProject} from './IProject'
import type {PrefixMode} from '@/modules/parseTaskText'
import type {BasicColorSchema} from '@vueuse/core'
import type {SupportedLocale} from '@/i18n'
import type {DefaultProjectViewKind} from '@/modelTypes/IProjectView'
export interface IFrontendSettings {
playSoundWhenDone: boolean
quickAddMagicMode: PrefixMode
colorSchema: BasicColorSchema
filterIdUsedOnOverview: IProject['id'] | null
defaultView?: DefaultProjectViewKind
}
export interface IUserSettings extends IAbstract {

View File

@ -3,6 +3,7 @@ import AbstractModel from './abstractModel'
import type {IFrontendSettings, IUserSettings} from '@/modelTypes/IUserSettings'
import {getBrowserLanguage} from '@/i18n'
import {PrefixMode} from '@/modules/parseTaskText'
import {DEFAULT_PROJECT_VIEW_SETTINGS} from '@/modelTypes/IProjectView'
export default class UserSettingsModel extends AbstractModel<IUserSettings> implements IUserSettings {
name = ''
@ -19,6 +20,7 @@ export default class UserSettingsModel extends AbstractModel<IUserSettings> impl
playSoundWhenDone: true,
quickAddMagicMode: PrefixMode.Default,
colorSchema: 'auto',
defaultView: DEFAULT_PROJECT_VIEW_SETTINGS.FIRST,
konrad marked this conversation as resolved Outdated

Shouldn't this be DEFAULT_PROJECT_VIEW_SETTINGS.FIRST?

Shouldn't this be `DEFAULT_PROJECT_VIEW_SETTINGS.FIRST`?

Yeah, good spot

Yeah, good spot
}
constructor(data: Partial<IUserSettings> = {}) {

View File

@ -8,6 +8,8 @@ import ProjectList from '@/components/project/views/ProjectList.vue'
import ProjectGantt from '@/components/project/views/ProjectGantt.vue'
import ProjectTable from '@/components/project/views/ProjectTable.vue'
import ProjectKanban from '@/components/project/views/ProjectKanban.vue'
import {useAuthStore} from '@/stores/auth'
import {DEFAULT_PROJECT_VIEW_SETTINGS} from '@/modelTypes/IProjectView'
const {
projectId,
@ -19,6 +21,7 @@ const {
const router = useRouter()
const projectStore = useProjectStore()
const authStore = useAuthStore()
const currentView = computed(() => {
const project = projectStore.projects[projectId]
@ -26,17 +29,27 @@ const currentView = computed(() => {
return project?.views.find(v => v.id === viewId)
})
function redirectToFirstViewIfNecessary() {
function redirectToDefaultViewIfNecessary() {
if (viewId === 0 || !projectStore.projects[projectId]?.views.find(v => v.id === viewId)) {
// Ideally, we would do that in the router redirect, but the projects (and therefore, the views)
// are not always loaded then.
const firstViewId = projectStore.projects[projectId]?.views[0].id
if (firstViewId) {
let view
if (authStore.settings.frontendSettings.defaultView !== DEFAULT_PROJECT_VIEW_SETTINGS.FIRST) {
Review

Nicely done!

Nicely done!
view = projectStore.projects[projectId]?.views.find(v => v.viewKind === authStore.settings.frontendSettings.defaultView)
}
konrad marked this conversation as resolved Outdated

This won't work. What if I rearrange the views or delete a view so that, for example, the kanban view is at the first position? Then this will crash.

Please loop over projectStore.projects[projectId]?.views until you find the first view of the kind that's saved in authStore.settings.frontendSettings.defaultView. If that's not available (use the .find function, then the result here is undefined), use the first view, in the same way as it was checked previously.

This won't work. What if I rearrange the views or delete a view so that, for example, the kanban view is at the first position? Then this will crash. Please loop over `projectStore.projects[projectId]?.views` until you find the first view of the kind that's saved in `authStore.settings.frontendSettings.defaultView`. If that's not available (use the `.find` function, then the result here is `undefined`), use the first view, in the same way as it was checked previously.

I'm not quite sure I did what you wanted, but it should search for them by view kind now.

I still couldn't test it as the views are always undefined even in the main branch for me and it always results in projectStore.projects[__props.projectId].views[something] is undefined when opening a project (opening tasks directly works).

I'm not quite sure I did what you wanted, but it should search for them by view kind now. I still couldn't test it as the views are always undefined even in the main branch for me and it always results in `projectStore.projects[__props.projectId].views[something] is undefined` when opening a project (opening tasks directly works).

I still couldn't test it as the views are always undefined even in the main branch for me

Did you test with an api built from the main branch as well?

> I still couldn't test it as the views are always undefined even in the main branch for me Did you test with an api built from the main branch as well?

Yes I did.

Yes I did.

Update: I actually just got it working - it is likely related to updating the backend that I used for testing.

Update: I actually just got it working - it is likely related to updating the backend that I used for testing.

So it all works fine?

So it all works fine?

Yeah I just tried it. Works just fine!
Now the only thing I'll be looking into adding is a default setting in the configs. (Although it looks like currently no frontend settings support default config options - would that make sense to add here?)

Yeah I just tried it. Works just fine! Now the only thing I'll be looking into adding is a default setting in the configs. (Although it looks like currently no frontend settings support default config options - would that make sense to add here?)

You mean a default setting in the config for new users? TBH I don't think that's needed because it is easily changeable for users.

You mean a default setting in the config for new users? TBH I don't think that's needed because it is easily changeable for users.

The reason why I'm bringing this up is that some of our members were very confused about the default list layout, and they didn't see the option to change the view to kanban which is what they're used to. A default setting would help with that.

The reason why I'm bringing this up is that some of our members were very confused about the default list layout, and they didn't see the option to change the view to kanban which is what they're used to. A default setting would help with that.

Okay, but doesn't educating the users about the different views and how they can set a default for them (with this PR) solve that problem?

Okay, but doesn't educating the users about the different views and how they can set a default for them (with this PR) solve that problem?

It does! I just wanted to have something for their convenience to set it to this by default.

It does! I just wanted to have something for their convenience to set it to this by default.
// Use the first view as fallback if the default view is not available
if (view === undefined && projectStore.projects[projectId]?.views?.length > 0) {
view = projectStore.projects[projectId]?.views[0]
}
if (view) {
router.replace({
name: 'project.view',
params: {
projectId,
viewId: firstViewId,
viewId: view.id,
},
})
}
@ -45,13 +58,13 @@ function redirectToFirstViewIfNecessary() {
watch(
() => viewId,
redirectToFirstViewIfNecessary,
redirectToDefaultViewIfNecessary,
{immediate: true},
)
watch(
() => projectStore.projects[projectId],
redirectToFirstViewIfNecessary,
redirectToDefaultViewIfNecessary,
)
// using a watcher instead of beforeEnter because beforeEnter is not called when only the viewId changes

View File

@ -26,6 +26,22 @@
</label>
<ProjectSearch v-model="defaultProject" />
</div>
<div class="field">
<label class="label">
{{ $t('user.settings.general.defaultView') }}
konrad marked this conversation as resolved Outdated

There should be an explanation here somewhere about what the default is when no view is configured. This could be a disabled first option in the select or a helper text.

There should be an explanation here somewhere about what the default is when no view is configured. This could be a disabled first option in the select or a helper text.

When the default view setting is not configured then it will default to list, I think that should be intuitive as this setting is also list by default.

For there case where there is for example no Kanban view for the project when Kanban is the default view type, then it should default to the first view in the list, I've added this in another commit.

When the default view setting is not configured then it will default to list, I think that should be intuitive as this setting is also list by default. For there case where there is for example no Kanban view for the project when Kanban is the default view type, then it should default to the first view in the list, I've added this in another commit.

Gotcha, please document this in the settings dialogue.

Gotcha, please document this in the settings dialogue.

When the default view setting is not configured then it will default to list, I think that should be intuitive as this setting is also list by default.

Do you mean I should document this in the code or add a tooltip? I kind of think a tooltip wouldn't make much sense as the setting is on list anyway, then it is clear that this is the default view.

> When the default view setting is not configured then it will default to list, I think that should be intuitive as this setting is also list by default. Do you mean I should document this in the code or add a tooltip? I kind of think a tooltip wouldn't make much sense as the setting is on list anyway, then it is clear that this is the default view.

Ah, now I understood what you mean! I was unable to verify this though, it still looks like this when no default view is saved:

image

It should preselect "list" in that dropdown.

Ah, now I understood what you mean! I was unable to verify this though, it still looks like this when no default view is saved: ![image](/attachments/4c38f9b4-ab15-4e3d-af5a-3d6c238a7752) It should preselect "list" in that dropdown.

Can you send the image again I can't see it.
What's the defaultView value for you when nothing is set? Clearing cookies doesn't seem to remove it for me.

Can you send the image again I can't see it. What's the defaultView value for you when nothing is set? Clearing cookies doesn't seem to remove it for me.

Image: https://kolaente.dev/vikunja/vikunja/attachments/4c38f9b4-ab15-4e3d-af5a-3d6c238a7752

What's the defaultView value for you when nothing is set? Clearing cookies doesn't seem to remove it for me.

I've used this with an account which existed before the change, hence it does not have the value set in the frontend settings (it's not empty, the property does not exist) - in the users table, column frontend_settings.

Image: https://kolaente.dev/vikunja/vikunja/attachments/4c38f9b4-ab15-4e3d-af5a-3d6c238a7752 > What's the defaultView value for you when nothing is set? Clearing cookies doesn't seem to remove it for me. I've used this with an account which existed before the change, hence it does not have the value set in the frontend settings (it's not empty, the property does not exist) - in the `users` table, column `frontend_settings`.

It still stays Not Found for this link, but I've added a bit that should fix this issue.

It still stays Not Found for this link, but I've added a bit that should fix this issue.

Now I get this error when I open the settings page:

main.ts:72 TypeError: 'set' on proxy: trap returned falsish for property 'settings'
    at setup (General.vue:276:12)
    at callWithErrorHandling (chunk-UG4MLOHL.js?v=6e94c7bd:1867:19)
    at setupStatefulComponent (chunk-UG4MLOHL.js?v=6e94c7bd:8183:25)
    at setupComponent (chunk-UG4MLOHL.js?v=6e94c7bd:8144:36)
    at mountComponent (chunk-UG4MLOHL.js?v=6e94c7bd:6731:7)
    at processComponent (chunk-UG4MLOHL.js?v=6e94c7bd:6697:9)
    at patch (chunk-UG4MLOHL.js?v=6e94c7bd:6163:11)
    at ReactiveEffect.componentUpdateFn [as fn] (chunk-UG4MLOHL.js?v=6e94c7bd:6841:11)
    at ReactiveEffect.run (chunk-UG4MLOHL.js?v=6e94c7bd:1410:23)
    at instance.update (chunk-UG4MLOHL.js?v=6e94c7bd:6965:17) 
Proxy(Object)
 'setup function'
main.ts:72 TypeError: Cannot read properties of undefined (reading '_withKeys')
    at withKeys (chunk-UG4MLOHL.js?v=6e94c7bd:11584:24)
    at General.vue:1:1
    at renderFnWithContext (chunk-UG4MLOHL.js?v=6e94c7bd:2436:13)
    at renderSlot (chunk-UG4MLOHL.js?v=6e94c7bd:4133:53)
    at Proxy._sfc_render (card.vue:33:5)
    at renderComponentRoot (chunk-UG4MLOHL.js?v=6e94c7bd:2494:17)
    at ReactiveEffect.componentUpdateFn [as fn] (chunk-UG4MLOHL.js?v=6e94c7bd:6834:46)
    at ReactiveEffect.run (chunk-UG4MLOHL.js?v=6e94c7bd:1410:23)
    at instance.update (chunk-UG4MLOHL.js?v=6e94c7bd:6965:17)
    at setupRenderEffect (chunk-UG4MLOHL.js?v=6e94c7bd:6975:5) 
Proxy(Object)
 'render function'


Now I get this error when I open the settings page: ``` main.ts:72 TypeError: 'set' on proxy: trap returned falsish for property 'settings' at setup (General.vue:276:12) at callWithErrorHandling (chunk-UG4MLOHL.js?v=6e94c7bd:1867:19) at setupStatefulComponent (chunk-UG4MLOHL.js?v=6e94c7bd:8183:25) at setupComponent (chunk-UG4MLOHL.js?v=6e94c7bd:8144:36) at mountComponent (chunk-UG4MLOHL.js?v=6e94c7bd:6731:7) at processComponent (chunk-UG4MLOHL.js?v=6e94c7bd:6697:9) at patch (chunk-UG4MLOHL.js?v=6e94c7bd:6163:11) at ReactiveEffect.componentUpdateFn [as fn] (chunk-UG4MLOHL.js?v=6e94c7bd:6841:11) at ReactiveEffect.run (chunk-UG4MLOHL.js?v=6e94c7bd:1410:23) at instance.update (chunk-UG4MLOHL.js?v=6e94c7bd:6965:17) Proxy(Object) 'setup function' main.ts:72 TypeError: Cannot read properties of undefined (reading '_withKeys') at withKeys (chunk-UG4MLOHL.js?v=6e94c7bd:11584:24) at General.vue:1:1 at renderFnWithContext (chunk-UG4MLOHL.js?v=6e94c7bd:2436:13) at renderSlot (chunk-UG4MLOHL.js?v=6e94c7bd:4133:53) at Proxy._sfc_render (card.vue:33:5) at renderComponentRoot (chunk-UG4MLOHL.js?v=6e94c7bd:2494:17) at ReactiveEffect.componentUpdateFn [as fn] (chunk-UG4MLOHL.js?v=6e94c7bd:6834:46) at ReactiveEffect.run (chunk-UG4MLOHL.js?v=6e94c7bd:1410:23) at instance.update (chunk-UG4MLOHL.js?v=6e94c7bd:6965:17) at setupRenderEffect (chunk-UG4MLOHL.js?v=6e94c7bd:6975:5) Proxy(Object) 'render function' 

Oops, try now

Oops, try now
</label>
<div class="select">
Elscrux marked this conversation as resolved Outdated

This should not have margin left.

This should not have margin left.
<select v-model="settings.frontendSettings.defaultView">
<option
v-for="view in DEFAULT_PROJECT_VIEW_SETTINGS"
:key="view"
:value="view"
>
{{ $t(`project.${view}.title`) }}
</option>
</select>
</div>
</div>
<div
v-if="hasFilters"
class="field"
@ -221,6 +237,7 @@ import {useProjectStore} from '@/stores/projects'
import {useAuthStore} from '@/stores/auth'
import type {IUserSettings} from '@/modelTypes/IUserSettings'
import {isSavedFilter} from '@/services/savedFilter'
import {DEFAULT_PROJECT_VIEW_SETTINGS} from '@/modelTypes/IProjectView'
const {t} = useI18n({useScope: 'global'})
useTitle(() => `${t('user.settings.general.title')} - ${t('user.settings.title')}`)
@ -253,12 +270,15 @@ function useAvailableTimezones() {
const availableTimezones = useAvailableTimezones()
const authStore = useAuthStore()
const settings = ref<IUserSettings>({
...authStore.settings,
konrad marked this conversation as resolved Outdated

This won't work. If you want code to run when the component is mounted or created, use one of the lifecycle hooks.

However, rethinking this I'm not sure if this should be the list view in the dropdown by default since it will use the first view which may not be the list view but something else entirely.
Let's instead do the following: When no view is set (v-if="!settings.frontendSettings.defaultView"), show a text near to the dropdown stating "will use the first view since none is configured".

This won't work. If you want code to run when the component is mounted or created, use [one of the lifecycle hooks](https://vuejs.org/guide/essentials/lifecycle.html). However, rethinking this I'm not sure if this should be the list view in the dropdown by default since it will use the *first* view which may not be the list view but something else entirely. Let's instead do the following: When no view is set (`v-if="!settings.frontendSettings.defaultView"`), show a text near to the dropdown stating "will use the first view since none is configured".

Wouldn't it be simpler to just set the setting to List by default when it's not set yet? No need to create this edge case which is only relevant for migrating from an older version.

Wouldn't it be simpler to just set the setting to List by default when it's not set yet? No need to create this edge case which is only relevant for migrating from an older version.

Well there's a difference in logic since it will use the first view if no default view is configured. Maybe people want to keep it that way and never change the setting.

Well there's a difference in logic since it will use the first view if no default view is configured. Maybe people want to keep it that way and never change the setting.

Yes, but this difference wouldn't be there if the setting is just set to List by default when no setting is currently set. When you start a Vikunja instance with this feature implemented, it will always pick List, no? For updating from older ones, this should automatically be set to be on the same state.
Otherwise if you really want to give people to option to use the first view as option, then this should be an option you can select in the settings, and not something that is not retrievable once you set this once.

Yes, but this difference wouldn't be there if the setting is just set to List by default when no setting is currently set. When you start a Vikunja instance with this feature implemented, it will always pick List, no? For updating from older ones, this should automatically be set to be on the same state. Otherwise if you really want to give people to option to use the first view as option, then this should be an option you can select in the settings, and not something that is not retrievable once you set this once.

When you start a Vikunja instance with this feature implemented, it will always pick List, no?

For new projects, it will always be list but only as long as the list view exists and is the first view. It's not "the list view" but "the first view".

Otherwise if you really want to give people to option to use the first view as option, then this should be an option you can select in the settings, and not something that is not retrievable once you set this once.

Agreed. Can you add this?

> When you start a Vikunja instance with this feature implemented, it will always pick List, no? For new projects, it will always be list but only as long as the list view exists and is the first view. It's not "the list view" but "the first view". > Otherwise if you really want to give people to option to use the first view as option, then this should be an option you can select in the settings, and not something that is not retrievable once you set this once. Agreed. Can you add this?

Sure! I've pushed an update for this. Not sure that's the best way to do it but it should work fine.

Sure! I've pushed an update for this. Not sure that's the best way to do it but it should work fine.
frontendSettings: {
// Sub objects get exported as read only as well, so we need to
// explicitly spread the object here to allow modification
...authStore.settings.frontendSettings,
// Add fallback for old settings that don't have the default view set
defaultView: authStore.settings.frontendSettings.defaultView ?? DEFAULT_PROJECT_VIEW_SETTINGS.FIRST,
konrad marked this conversation as resolved Outdated

This should be DEFAULT_PROJECT_VIEW_SETTINGS.FIRST as well

This should be `DEFAULT_PROJECT_VIEW_SETTINGS.FIRST` as well
},
})
const id = ref(createRandomID())