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

feat: enforce prefix for persisted route query values #9226

Merged
merged 3 commits into from
Jun 15, 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
5 changes: 5 additions & 0 deletions changelog/unreleased/enhancement-streamline-url-query-params
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Enhancement: Streamline URL query names

We've used different URL query names for the pagination in the files app (`items-per-page`) and admin-settings app (`admin-settings-items-per-page`). We've streamlined this to use the same query name in all apps and still keep the possibility to have independent page sizes in different apps.

https://github.com/owncloud/web/pull/9226
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
:has-file-extensions="false"
:has-pagination="true"
:pagination-options="paginationOptions"
:per-page-query-name="perPageQueryName"
:per-page-default="perPageDefault"
per-page-storage-prefix="admin-settings"
/>
<oc-button
v-if="sideBarAvailablePanels.length"
Expand Down Expand Up @@ -67,11 +67,7 @@
</template>

<script lang="ts">
import {
perPageQueryName,
perPageDefault,
paginationOptions
} from 'web-app-admin-settings/src/defaults'
import { perPageDefault, paginationOptions } from 'web-app-admin-settings/src/defaults'
import AppLoadingSpinner from 'web-pkg/src/components/AppLoadingSpinner.vue'
import SideBar from 'web-pkg/src/components/SideBar/SideBar.vue'
import BatchActions from 'web-pkg/src/components/BatchActions.vue'
Expand Down Expand Up @@ -201,7 +197,6 @@ export default defineComponent({
...useAppDefaults({
applicationId: 'admin-settings'
}),
perPageQueryName,
perPageDefault,
paginationOptions
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ import { useGettext } from 'vue3-gettext'
import { defaultFuseOptions } from 'web-pkg/src/helpers'
import { useFileListHeaderPosition, usePagination } from 'web-pkg/src/composables'
import Pagination from 'web-pkg/src/components/Pagination.vue'
import { perPageDefault, perPageQueryName } from 'web-app-admin-settings/src/defaults'
import { perPageDefault, perPageStoragePrefix } from 'web-app-admin-settings/src/defaults'

export default defineComponent({
name: 'GroupsList',
Expand Down Expand Up @@ -228,7 +228,7 @@ export default defineComponent({
items: paginatedItems,
page: currentPage,
total: totalPages
} = usePagination({ items, perPageDefault, perPageQueryName })
} = usePagination({ items, perPageDefault, perPageStoragePrefix })

watch(currentPage, () => {
emit('unSelectAllGroups')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ import {
usePagination
} from 'web-pkg/src/composables'
import Pagination from 'web-pkg/src/components/Pagination.vue'
import { perPageDefault, perPageQueryName } from 'web-app-admin-settings/src/defaults'
import { perPageDefault, perPageStoragePrefix } from 'web-app-admin-settings/src/defaults'

export default defineComponent({
name: 'SpacesList',
Expand Down Expand Up @@ -229,7 +229,7 @@ export default defineComponent({
items: paginatedItems,
page: currentPage,
total: totalPages
} = usePagination({ items, perPageDefault, perPageQueryName })
} = usePagination({ items, perPageDefault, perPageStoragePrefix })

watch(currentPage, () => {
emit('unSelectAllSpaces')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ import NoContentMessage from 'web-pkg/src/components/NoContentMessage.vue'
import { useFileListHeaderPosition, usePagination } from 'web-pkg/src/composables'
import Pagination from 'web-pkg/src/components/Pagination.vue'
import { computed } from 'vue'
import { perPageDefault, perPageQueryName } from 'web-app-admin-settings/src/defaults'
import { perPageDefault, perPageStoragePrefix } from 'web-app-admin-settings/src/defaults'

export default defineComponent({
name: 'UsersList',
Expand Down Expand Up @@ -265,7 +265,7 @@ export default defineComponent({
items: paginatedItems,
page: currentPage,
total: totalPages
} = usePagination({ items, perPageDefault, perPageQueryName })
} = usePagination({ items, perPageDefault, perPageStoragePrefix })

watch(currentPage, () => {
emit('unSelectAllUsers')
Expand Down
2 changes: 1 addition & 1 deletion packages/web-app-admin-settings/src/defaults/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export const supportedLogoMimeTypes = ['image/gif', 'image/jpeg', 'image/png']
export const perPageDefault = '50'
export const perPageQueryName = 'admin-settings-items-per-page'
export const perPageStoragePrefix = 'admin-settings'
export const paginationOptions = ['20', '50', '100', '250']
1 change: 1 addition & 0 deletions packages/web-app-files/src/components/AppBar/AppBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
:has-hidden-files="hasHiddenFiles"
:has-file-extensions="hasFileExtensions"
:has-pagination="hasPagination"
per-page-storage-prefix="files"
/>
<sidebar-toggle v-if="hasSidebarToggle" :side-bar-open="sideBarOpen" />
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const useResourcesViewDefaults = <T, TT, TU extends any[]>(
items: paginatedResources,
total: paginationPages,
page: paginationPage
} = usePagination({ items })
} = usePagination({ items, perPageStoragePrefix: 'files' })

useMutationSubscription(['Files/UPSERT_RESOURCE'], async ({ payload }) => {
await nextTick()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`AppBar component renders by default no breadcrumbs, no bulkactions, no
<portal-target name="app.runtime.mobile.nav"></portal-target>
<!--v-if-->
<div class="oc-flex" id="files-app-bar-controls-right">
<view-options-stub hasfileextensions="true" hashiddenfiles="true" haspagination="true" paginationoptions="20,50,100,250,500" perpagedefault="100" perpagequeryname="items-per-page" viewmodes=""></view-options-stub>
<view-options-stub hasfileextensions="true" hashiddenfiles="true" haspagination="true" paginationoptions="20,50,100,250,500" perpagedefault="100" perpagequeryname="items-per-page" perpagestorageprefix="files" viewmodes=""></view-options-stub>
<sidebar-toggle-stub sidebaropen="false"></sidebar-toggle-stub>
</div>
</div>
Expand All @@ -33,7 +33,7 @@ exports[`AppBar component renders if given, with content in the actions slot 1`]
<portal-target name="app.runtime.mobile.nav"></portal-target>
<!--v-if-->
<div class="oc-flex" id="files-app-bar-controls-right">
<view-options-stub hasfileextensions="true" hashiddenfiles="true" haspagination="true" paginationoptions="20,50,100,250,500" perpagedefault="100" perpagequeryname="items-per-page" viewmodes=""></view-options-stub>
<view-options-stub hasfileextensions="true" hashiddenfiles="true" haspagination="true" paginationoptions="20,50,100,250,500" perpagedefault="100" perpagequeryname="items-per-page" perpagestorageprefix="files" viewmodes=""></view-options-stub>
<sidebar-toggle-stub sidebaropen="false"></sidebar-toggle-stub>
</div>
</div>
Expand All @@ -57,7 +57,7 @@ exports[`AppBar component renders if given, with content in the content slot 1`]
<portal-target name="app.runtime.mobile.nav"></portal-target>
<!--v-if-->
<div class="oc-flex" id="files-app-bar-controls-right">
<view-options-stub hasfileextensions="true" hashiddenfiles="true" haspagination="true" paginationoptions="20,50,100,250,500" perpagedefault="100" perpagequeryname="items-per-page" viewmodes=""></view-options-stub>
<view-options-stub hasfileextensions="true" hashiddenfiles="true" haspagination="true" paginationoptions="20,50,100,250,500" perpagedefault="100" perpagequeryname="items-per-page" perpagestorageprefix="files" viewmodes=""></view-options-stub>
<sidebar-toggle-stub sidebaropen="false"></sidebar-toggle-stub>
</div>
</div>
Expand Down
7 changes: 6 additions & 1 deletion packages/web-pkg/src/components/ViewOptions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ export default defineComponent({
type: String,
default: PaginationConstants.perPageDefault
},
perPageStoragePrefix: {
type: String,
required: true
},
viewModes: {
type: Array as PropType<ViewMode[]>,
default: () => []
Expand All @@ -141,7 +145,8 @@ export default defineComponent({
})
const itemsPerPageQuery = useRouteQueryPersisted({
name: props.perPageQueryName,
defaultValue: props.perPageDefault
defaultValue: props.perPageDefault,
storagePrefix: props.perPageStoragePrefix
})
const viewModeQuery = useRouteQueryPersisted({
name: ViewModeConstants.queryName,
Expand Down
12 changes: 9 additions & 3 deletions packages/web-pkg/src/composables/pagination/usePagination.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import { computed, ComputedRef, unref } from 'vue'
import { MaybeRef } from 'web-pkg/src/utils'
import { queryItemAsString, useRouteQuery, useRouteQueryPersisted } from 'web-pkg/src/composables'
import { PaginationConstants } from './constants'
import {
queryItemAsString,
useRouteQuery,
useRouteQueryPersisted,
PaginationConstants
} from 'web-pkg/src/composables'

interface PaginationOptions<T> {
items: MaybeRef<Array<T>>
page?: MaybeRef<number>
perPage?: MaybeRef<number>
perPageDefault?: string
perPageQueryName?: string
perPageStoragePrefix: string
}

interface PaginationResult<T> {
Expand Down Expand Up @@ -59,7 +64,8 @@ function createPerPageRef<T>(options: PaginationOptions<T>): ComputedRef<number>

const perPageQuery = useRouteQueryPersisted({
name: options.perPageQueryName || PaginationConstants.perPageQueryName,
defaultValue: options.perPageDefault || PaginationConstants.perPageDefault
defaultValue: options.perPageDefault || PaginationConstants.perPageDefault,
storagePrefix: options.perPageStoragePrefix
})
return computed(() => parseInt(queryItemAsString(unref(perPageQuery))))
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { Ref, watch, unref } from 'vue'
import { useRouteQuery } from './useRouteQuery'
import { useLocalStorage } from '../localStorage/useLocalStorage'
import { QueryValue } from './types'
import { useLocalStorage } from '../localStorage'

export interface RouteQueryPersistedOptions {
name: string
defaultValue: QueryValue
routeName?: string
storagePrefix?: string
}

interface WatcherValue {
Expand Down Expand Up @@ -50,5 +50,5 @@ export const useRouteQueryPersisted = (options: RouteQueryPersistedOptions): Ref
}

const localStorageKey = (options: RouteQueryPersistedOptions): string => {
return ['oc_options', options.routeName, options.name].filter(Boolean).join('_')
return ['oc-options', options.storagePrefix, options.name].filter(Boolean).join('_')
}
4 changes: 2 additions & 2 deletions packages/web-pkg/src/composables/sort/useSort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function createSortByQueryRef<T>(options: SortOptions<T>): Ref<QueryValue> {
return useRouteQueryPersisted({
name: unref(options.sortByQueryName) || SortConstants.sortByQueryName,
defaultValue: unref(firstSortableField(unref(options.fields))?.name),
routeName: unref(options.routeName) || unref(useRouteName())
storagePrefix: unref(options.routeName) || unref(useRouteName())
})
}

Expand All @@ -108,7 +108,7 @@ function createSortDirQueryRef<T>(options: SortOptions<T>): Ref<QueryValue> {
return useRouteQueryPersisted({
name: unref(options.sortDirQueryName) || SortConstants.sortDirQueryName,
defaultValue: unref(firstSortableField(unref(options.fields))?.sortDir),
routeName: unref(options.routeName) || unref(useRouteName())
storagePrefix: unref(options.routeName) || unref(useRouteName())
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ function getWrapper({
const instance = usePagination({
items: ref(items),
page: currentPage,
perPage: itemsPerPage
perPage: itemsPerPage,
perPageStoragePrefix: 'unit-tests'
})
setup(instance)
})
Expand Down