Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix pagination after increasing items per page #8854

Merged
merged 2 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Pagination after increasing items per page

An issue where the file list incorrectly showed no items after paginating and increasing the amount of items per page has been fixed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An issue where the file list incorrectly showed no items after paginating and increasing the amount of items per page has been fixed.
An issue where the file list incorrectly showed no items after paginating and changing the amount of items per page has been fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was about to do the same, but the bug report is still right though, no? The issue only occurred when increasing the amount of items.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes :D fine by me either way


https://github.com/owncloud/web/issues/6768
https://github.com/owncloud/web/pull/8854
57 changes: 40 additions & 17 deletions packages/web-app-files/src/components/AppBar/ViewOptions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
v-if="viewModes.includes(ViewModeConstants.tilesView)"
class="files-view-options-list-item oc-visible@s oc-flex oc-flex-between oc-flex-middle"
>
<label for="tiles-size-slider" v-text="resizeTilesLabel" />
<label for="tiles-size-slider" v-text="$gettext('Tile size')" />
<input
v-model="viewSizeCurrent"
type="range"
Expand All @@ -89,9 +89,16 @@
</template>

<script lang="ts">
import { defineComponent, PropType, ref, unref, watch } from 'vue'
import { computed, defineComponent, PropType, ref, unref, watch } from 'vue'
import { mapMutations, mapState } from 'vuex'
import { queryItemAsString, useRouteQueryPersisted } from 'web-pkg/src/composables'
import { useGettext } from 'vue3-gettext'
import {
queryItemAsString,
useRoute,
useRouteQuery,
useRouteQueryPersisted,
useRouter
} from 'web-pkg/src/composables'
import { ViewMode } from 'web-pkg/src/ui/types'
import { PaginationConstants, ViewModeConstants } from '../../composables'

Expand All @@ -106,8 +113,19 @@ export default defineComponent({
}
},
setup() {
const router = useRouter()
const currentRoute = useRoute()
const { $gettext } = useGettext()

const queryParamsLoading = ref(false)

const currentPageQuery = useRouteQuery('page')
const currentPage = computed(() => {
if (!unref(currentPageQuery)) {
return 1
}
return parseInt(queryItemAsString(unref(currentPageQuery)))
})
const itemsPerPageQuery = useRouteQueryPersisted({
name: PaginationConstants.perPageQueryName,
defaultValue: PaginationConstants.perPageDefault
Expand All @@ -130,6 +148,20 @@ export default defineComponent({
}
}

const setItemsPerPage = (itemsPerPage: string) => {
return router.replace({
query: {
...unref(currentRoute).query,
'items-per-page': itemsPerPage,
...(unref(currentPage) > 1 && { page: '1' })
}
})
}

const setViewMode = (mode: ViewMode) => {
viewModeQuery.value = mode.name
}

watch(
[itemsPerPageQuery, viewModeQuery, viewSizeQuery],
(params) => {
Expand All @@ -154,19 +186,15 @@ export default defineComponent({
viewSizeCurrent: viewSizeQuery,
itemsPerPageCurrent: itemsPerPageQuery,
queryParamsLoading,
setTilesViewSize
setTilesViewSize,
setItemsPerPage,
setViewMode,
viewOptionsButtonLabel: $gettext('Display customization options of the files list')
}
},
computed: {
...mapState('Files', ['areHiddenFilesShown', 'areFileExtensionsShown']),

viewOptionsButtonLabel() {
return this.$gettext('Display customization options of the files list')
},
resizeTilesLabel() {
return this.$gettext('Tile size')
},

hiddenFilesShownModel: {
get() {
return this.areHiddenFilesShown
Expand All @@ -189,12 +217,7 @@ export default defineComponent({
methods: {
queryItemAsString,
...mapMutations('Files', ['SET_HIDDEN_FILES_VISIBILITY', 'SET_FILE_EXTENSIONS_VISIBILITY']),
setViewMode(mode) {
this.viewModeCurrent = mode.name
},
setItemsPerPage(itemsPerPage) {
this.itemsPerPageCurrent = itemsPerPage
},

updateHiddenFilesShownModel(event) {
this.hiddenFilesShownModel = event
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
import ViewOptions from 'web-app-files/src/components/AppBar/ViewOptions.vue'
import { useRouteQueryPersisted } from 'web-pkg/src/composables/router'
import { ref, nextTick } from 'vue'
import { useRouteQueryPersisted, useRouteQuery } from 'web-pkg/src/composables/router'
import { ref } from 'vue'
import {
createStore,
defaultPlugins,
defaultStoreMockOptions,
defaultComponentMocks,
mount
mount,
RouteLocation
} from 'web-test-helpers'
import { ViewModeConstants } from 'web-app-files/src/composables'
import { mock } from 'jest-mock-extended'

jest.mock('web-pkg/src/composables/router', () => ({
...jest.requireActual('web-pkg/src/composables/router'),
useRouteQueryPersisted: jest.fn(),
useRouteQuery: jest.fn()
}))

jest.mock('web-pkg/src/composables/router')
const selectors = {
pageSizeSelect: '.oc-page-size',
hiddenFilesSwitch: '[data-testid="files-switch-hidden-files"]',
Expand All @@ -30,14 +37,26 @@ describe('ViewOptions component', () => {
const { wrapper } = getWrapper({ perPage })
expect(wrapper.findComponent<any>(selectors.pageSizeSelect).props().selected).toBe(perPage)
})
it('sets the correct files page limit', async () => {
it('sets the correct files page limit', () => {
const perPage = '100'
const newItemsPerPage = '500'
const { wrapper } = getWrapper({ perPage })
const { wrapper, mocks } = getWrapper({ perPage })
wrapper.vm.setItemsPerPage(newItemsPerPage)
expect(mocks.$router.replace).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({ 'items-per-page': newItemsPerPage })
})
)
})
it('resets the page to 1 if current page is > 1', () => {
const perPage = '100'
const newItemsPerPage = '500'
const { wrapper, mocks } = getWrapper({ perPage, currentPage: '2' })
wrapper.vm.setItemsPerPage(newItemsPerPage)
await nextTick()
expect(wrapper.findComponent<any>(selectors.pageSizeSelect).props().selected).toBe(
newItemsPerPage
expect(mocks.$router.replace).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({ 'items-per-page': newItemsPerPage, page: '1' })
})
)
})
})
Expand Down Expand Up @@ -109,15 +128,19 @@ function getWrapper({
perPage = '100',
viewMode = ViewModeConstants.default.name,
tileSize = '1',
props = {}
props = {},
currentPage = '1'
} = {}) {
jest.mocked(useRouteQueryPersisted).mockImplementationOnce(() => ref(perPage))
jest.mocked(useRouteQueryPersisted).mockImplementationOnce(() => ref(viewMode))
jest.mocked(useRouteQueryPersisted).mockImplementationOnce(() => ref(tileSize))
jest.mocked(useRouteQuery).mockImplementationOnce(() => ref(currentPage))

const storeOptions = { ...defaultStoreMockOptions }
const store = createStore(storeOptions)
const mocks = { ...defaultComponentMocks() }
const mocks = {
...defaultComponentMocks({ currentRoute: mock<RouteLocation>({ path: '/files' }) })
}
return {
storeOptions,
mocks,
Expand Down