feat/alphabetical-sort #1162
|
@ -195,7 +195,7 @@ import NamespaceService from '@/services/namespace'
|
|||
import EditLabels from '@/components/tasks/partials/editLabels.vue'
|
||||
|
||||
import {objectToSnakeCase} from '@/helpers/case'
|
||||
import {getDefaultParams} from '../../tasks/mixins/taskList'
|
||||
import {getDefaultParams} from '@/components/tasks/mixins/taskList'
|
||||
dpschen marked this conversation as resolved
Outdated
|
||||
|
||||
// FIXME: merge with DEFAULT_PARAMS in taskList.js
|
||||
const DEFAULT_PARAMS = {
|
||||
|
@ -226,7 +226,7 @@ const DEFAULT_FILTERS = {
|
|||
namespace: '',
|
||||
}
|
||||
|
||||
const ALPHABETICAL_SORT = ['title']
|
||||
export const ALPHABETICAL_SORT = 'title'
|
||||
|
||||
export default {
|
||||
name: 'filters',
|
||||
|
@ -505,7 +505,7 @@ export default {
|
|||
this.params.sort_by = getDefaultParams().sort_by
|
||||
this.sortAlphabetically = false
|
||||
konrad marked this conversation as resolved
Outdated
konrad
commented
Please use a const for that. Please use a const for that.
|
||||
} else {
|
||||
this.params.sort_by = ALPHABETICAL_SORT
|
||||
this.params.sort_by = [ALPHABETICAL_SORT]
|
||||
this.sortAlphabetically = true
|
||||
}
|
||||
this.change()
|
||||
|
|
|
@ -148,6 +148,7 @@ import {HAS_TASKS} from '@/store/mutation-types'
|
|||
import Nothing from '@/components/misc/nothing.vue'
|
||||
import Pagination from '@/components/misc/pagination.vue'
|
||||
import Popup from '@/components/misc/popup'
|
||||
import { ALPHABETICAL_SORT } from '@/components/list/partials/filters'
|
||||
|
||||
import draggable from 'vuedraggable'
|
||||
import {calculateItemPosition} from '../../../helpers/calculateItemPosition'
|
||||
|
@ -228,7 +229,7 @@ export default {
|
|||
},
|
||||
methods: {
|
||||
isAlphabeticalSorting() {
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
Import declared const Import declared const `ALPHABETICAL_SORT`
|
||||
return this.sorting.find( sortBy => sortBy === 'title' ) !== undefined
|
||||
return this.sorting.find( sortBy => sortBy === ALPHABETICAL_SORT ) !== undefined
|
||||
dpschen marked this conversation as resolved
Outdated
dpschen
commented
Please use Please use `params.sort_by` => This way we do not need to introduce a new `this.sorting` variable in the mixin.
|
||||
},
|
||||
searchTasks() {
|
||||
// Only search if the search term changed
|
||||
|
@ -260,12 +261,22 @@ export default {
|
|||
focusNewTaskInput() {
|
||||
this.$refs.newTaskInput.$refs.newTaskInput.focus()
|
||||
},
|
||||
updateTaskList() {
|
||||
updateTaskList( task ) {
|
||||
if ( this.isAlphabeticalSorting() ) {
|
||||
dpschen marked this conversation as resolved
Outdated
konrad
commented
Still not quite happy with reloading everything. This gives a very noticable load flickering. I get why it is nessecary though. Maybe only reload if there are filters set and otherwise prepend the task as done already? Still not quite happy with reloading everything. This gives a very noticable load flickering. I get why it is nessecary though. Maybe only reload if there are filters set and otherwise prepend the task as done already?
dpschen
commented
The sorting should probably happen on client and server side. The sorting should probably happen on client _and_ server side.
This way the client can sort already what he is aware of and fetches an update.
konrad
commented
Since this would require a bit of work, I think we should do this in another PR and keep the current logic of prepending new tasks to the list. Since this would require a bit of work, I think we should do this in another PR and keep the current logic of prepending new tasks to the list.
Michaelpalacce
commented
So should I do reload only if filters and/or sorts are set ? So should I do reload only if filters and/or sorts are set ?
konrad
commented
I think that would be a good idea. I think that would be a good idea.
|
||||
this.reloadTasksWithCurrentFilterAndSorting()
|
||||
dpschen marked this conversation as resolved
dpschen
commented
Instead of putting this in a new function just make a comment:
Instead of putting this in a new function just make a comment:
```js
// reload tasks with current filter and sorting
this.loadTasks(1, undefined, undefined, true)
```
|
||||
}
|
||||
else {
|
||||
konrad marked this conversation as resolved
Outdated
konrad
commented
Why did you comment this? If it is not used, remove it. Why did you comment this? If it is not used, remove it.
|
||||
const tasks = [
|
||||
dpschen marked this conversation as resolved
dpschen
commented
Merge this:
Merge this:
```js
this.tasks = [
task,
...this.tasks,
]
```
|
||||
task,
|
||||
...this.tasks,
|
||||
]
|
||||
this.tasks = tasks
|
||||
konrad
commented
This should use the default params instead of This should use the default params instead of `undefined`.
Michaelpalacce
commented
Default parameters will overwrite the current filtering,searching etc, no? Idea here is to preserve it Default parameters will overwrite the current filtering,searching etc, no?
Idea here is to preserve it
konrad
commented
That's actually the idea behind the default paramters. If they are set, it will first look for the params in the data variables of the component and use them. The function parameters are only meant to override that. Passing in That's actually the idea behind the default paramters. If they are set, it will first look for the params in the data variables of the component and use them. The function parameters are only meant to override that.
Passing in `undefined` is, well, undefined behaviour.
Michaelpalacce
commented
Do you mean something like this:
Where DEFAULT PARAMS are:
Do you mean something like this:
reloadTasksWithCurrentFilterAndSorting(){
this.loadTasks(1, DEFAULT_PARAMS.s, DEFAULT_PARAMS, true)
},
Where DEFAULT PARAMS are:
~~~
// FIXME: merge with DEFAULT_PARAMS in taskList.js
export const DEFAULT_PARAMS = {
sort_by: [],
order_by: [],
filter_by: [],
filter_value: [],
filter_comparator: [],
filter_include_nulls: true,
filter_concat: 'or',
s: '',
}
~~~
konrad
commented
No, I mean the default parameters for the variables as they are declared in the function. You should call No, I mean the default parameters for the variables as they are declared in the function. You should call `this.loadTasks` with just the page function parameter specified (the first)
Michaelpalacce
commented
Sure but this would break the behavior since I need forceLoading ( the 4th parameter to be set ). undefined means take the default, it was my bad for passing undefined for page but otherwise it should be this.loadTasks(1, undefined, undefined, true) Sure but this would break the behavior since I need forceLoading ( the 4th parameter to be set ). undefined means take the default, it was my bad for passing undefined for page but otherwise it should be this.loadTasks(1, undefined, undefined, true)
konrad
commented
Please pass in Please pass in `''` for the search param and `null` for the params param so it matches the function declaration: https://kolaente.dev/vikunja/frontend/src/branch/main/src/components/tasks/mixins/taskList.js#L41-L46
|
||||
}
|
||||
|
||||
this.$store.commit(HAS_TASKS, true)
|
||||
this.reloadTasksWithCurrentFilterAndSorting()
|
||||
},
|
||||
reloadTasksWithCurrentFilterAndSorting(){
|
||||
this.loadTasks(undefined, undefined, undefined, true)
|
||||
this.loadTasks(1, undefined, undefined, true)
|
||||
},
|
||||
editTask(id) {
|
||||
// Find the selected task and set it to the current object
|
||||
|
|
Use
@