feat simplify taskList mixin #728
|
@ -11,7 +11,7 @@
|
|||
</x-button>
|
||||
</div>
|
||||
<filter-popup
|
||||
@change="loadTasks"
|
||||
@change="loadTasks()"
|
||||
konrad marked this conversation as resolved
Outdated
|
||||
:visible="showTaskFilter"
|
||||
v-model="params"
|
||||
/>
|
||||
|
|
|
@ -1,17 +1,60 @@
|
|||
import TaskCollectionService from '../../../services/taskCollection'
|
||||
import TaskCollectionService from '@/services/taskCollection'
|
||||
import cloneDeep from 'lodash/cloneDeep'
|
||||
import {calculateItemPosition} from '../../../helpers/calculateItemPosition'
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
Since the following two functions just calculate some result based on input I removed the definition of them from the mixin in preparation for future composition api. Since the following two functions just calculate some result based on input I removed the definition of them from the mixin in preparation for future composition api.
|
||||
|
||||
function createPagination(totalPages, currentPage) {
|
||||
const pages = []
|
||||
for (let i = 0; i < totalPages; i++) {
|
||||
|
||||
// Show ellipsis instead of all pages
|
||||
if (
|
||||
i > 0 && // Always at least the first page
|
||||
(i + 1) < totalPages && // And the last page
|
||||
(
|
||||
// And the current with current + 1 and current - 1
|
||||
(i + 1) > currentPage + 1 ||
|
||||
(i + 1) < currentPage - 1
|
||||
)
|
||||
) {
|
||||
// Only add an ellipsis if the last page isn't already one
|
||||
if (pages[i - 1] && !pages[i - 1].isEllipsis) {
|
||||
pages.push({
|
||||
number: 0,
|
||||
isEllipsis: true,
|
||||
})
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
pages.push({
|
||||
number: i + 1,
|
||||
isEllipsis: false,
|
||||
})
|
||||
}
|
||||
return pages
|
||||
}
|
||||
|
||||
export function getRouteForPagination(page = 1, type = 'list') {
|
||||
return {
|
||||
name: 'list.' + type,
|
||||
params: {
|
||||
type: type,
|
||||
},
|
||||
query: {
|
||||
page: page,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* This mixin provides a base set of methods and properties to get tasks on a list.
|
||||
*/
|
||||
export default {
|
||||
data() {
|
||||
return {
|
||||
taskCollectionService: TaskCollectionService,
|
||||
taskCollectionService: new TaskCollectionService(),
|
||||
tasks: [],
|
||||
|
||||
pages: [],
|
||||
currentPage: 0,
|
||||
|
||||
loadedList: null,
|
||||
|
@ -31,16 +74,17 @@ export default {
|
|||
}
|
||||
},
|
||||
watch: {
|
||||
'$route.query': 'loadTasksForPage', // Only listen for query path changes
|
||||
// Only listen for query path changes
|
||||
'$route.query': {
|
||||
handler: 'loadTasksForPage',
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
With With `immediate: true` the watcher is called on creation -> no additional call necessary in `beforeMount`, `created`, etc.
konrad
commented
Ohhh that's a really nice feature I didn't know. Ohhh that's a really nice feature I didn't know.
|
||||
immediate: true,
|
||||
},
|
||||
'$route.path': 'loadTasksOnSavedFilter',
|
||||
},
|
||||
beforeMount() {
|
||||
// Triggering loading the tasks in beforeMount lets the component maintain the current page, therefore the page
|
||||
// is not lost after navigating back from a task detail page for example.
|
||||
this.loadTasksForPage(this.$route.query)
|
||||
},
|
||||
created() {
|
||||
this.taskCollectionService = new TaskCollectionService()
|
||||
computed: {
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
By using a computed for By using a computed for `pages` we can spare us the hassle of keeping pages manually in sync and just let the automatic watcher do their job :)
|
||||
pages() {
|
||||
return createPagination(this.taskCollectionService.totalPages, this.currentPage)
|
||||
},
|
||||
},
|
||||
methods: {
|
||||
loadTasks(
|
||||
|
@ -80,48 +124,20 @@ export default {
|
|||
return
|
||||
}
|
||||
|
||||
this.$set(this, 'tasks', [])
|
||||
this.tasks = []
|
||||
|
||||
this.taskCollectionService.getAll(list, params, page)
|
||||
.then(r => {
|
||||
this.$set(this, 'tasks', r)
|
||||
this.$set(this, 'pages', [])
|
||||
this.tasks = r
|
||||
this.currentPage = page
|
||||
konrad marked this conversation as resolved
Outdated
dpschen
commented
In Vue 2 the In Vue 2 the `$set` function is just needed the for [change detection caveats](https://vuejs.org/v2/guide/reactivity.html#Change-Detection-Caveats). Directly replacing a reactive data value is not one of those. So no `$set` needed (we don't need it anywhere in vue 3).
konrad
commented
I see, I thought it was required for all things array mutations, not only directly replacing a value in an array. I see, I thought it was required for all things array mutations, not only directly replacing a value in an array.
|
||||
|
||||
for (let i = 0; i < this.taskCollectionService.totalPages; i++) {
|
||||
|
||||
// Show ellipsis instead of all pages
|
||||
if (
|
||||
i > 0 && // Always at least the first page
|
||||
(i + 1) < this.taskCollectionService.totalPages && // And the last page
|
||||
(
|
||||
// And the current with current + 1 and current - 1
|
||||
(i + 1) > this.currentPage + 1 ||
|
||||
(i + 1) < this.currentPage - 1
|
||||
)
|
||||
) {
|
||||
// Only add an ellipsis if the last page isn't already one
|
||||
if (this.pages[i - 1] && !this.pages[i - 1].isEllipsis) {
|
||||
this.pages.push({
|
||||
number: 0,
|
||||
isEllipsis: true,
|
||||
})
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
this.pages.push({
|
||||
number: i + 1,
|
||||
isEllipsis: false,
|
||||
})
|
||||
}
|
||||
|
||||
this.loadedList = cloneDeep(currentList)
|
||||
})
|
||||
.catch(e => {
|
||||
this.error(e)
|
||||
})
|
||||
},
|
||||
|
||||
loadTasksForPage(e) {
|
||||
// The page parameter can be undefined, in the case where the user loads a new list from the side bar menu
|
||||
let page = Number(e.page)
|
||||
|
@ -177,17 +193,6 @@ export default {
|
|||
this.showTaskSearch = false
|
||||
}, 200)
|
||||
},
|
||||
getRouteForPagination(page = 1, type = 'list') {
|
||||
return {
|
||||
name: 'list.' + type,
|
||||
params: {
|
||||
type: type,
|
||||
},
|
||||
query: {
|
||||
page: page,
|
||||
},
|
||||
}
|
||||
},
|
||||
saveTaskPosition(e) {
|
||||
this.drag = false
|
||||
|
||||
|
@ -205,5 +210,6 @@ export default {
|
|||
this.error(e)
|
||||
})
|
||||
},
|
||||
getRouteForPagination,
|
||||
},
|
||||
}
|
Keeping the parameters of the filter-popup change event in sync with the loadTasks parameters if error prone.
I'm not sure if it was intended here to provide the parameters (didn't check filterPopup).
AFAIK the event parameters are not used, I was just lazy when I added the change listener here.