Persist sorting per view #6290

kulmann · 2022-01-20T12:48:20Z

Description

This PR adds a new composable (useRouteQueryPersisted) which takes care of syncing non-default query params into the localStorage automatically. It has been hooked into the useSort and usePagination composables (optional, can be overwritten, e.g. with mocks for tests).

This kind of spams the localStorage a little bit, but since we want to be flexible regarding the persistence backend we probably need to be on a fine grained key-value-pair level with persisting options/settings.

With this we now persist the sort choices into the local storage per view. I.e. navigating into subfolders will keep the same sort choices. Navigating to different views will restore the historic sort choices of that respective view.

Related Issue

Fixes File sorting is not persistent #5930

Motivation and Context

Improve UX.

How Has This Been Tested?

Not yet, unit tests need to be added/adapted.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

changelog item
fix sorting on shared with me page

update-docs · 2022-01-20T12:48:24Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

kulmann · 2022-01-20T13:17:16Z

cc @dschmidt since this touches composables you have been working on

dschmidt

This is really nice!

When I introduced the sort composable, I was annoyed I had to make the same adjustments in several locations - this nicely gets rid of those duplications. Love it!

packages/web-app-files/src/composables/sort/useSort.ts

packages/web-app-files/src/composables/pagination/usePagination.ts

packages/web-pkg/src/composables/localStorage/useLocalStorage.ts

packages/web-pkg/src/composables/router/useRouteQueryPersisted.ts

pascalwengerter

Items per page seems to mostly work fine, just produced a This site appears to use a scroll-linked positioning effect. This may not work well with asynchronous panning; see https://firefox-source-docs.mozilla.org/performance/scroll-linked_effects.html for further details and to join the discussion on related tools and features! for me once in Firefox

Shared with me:

[Vue warn]: Property or method "targetRoute" is not defined on the instance but referenced during render. upon page navigation with only pending shares, pending&accepted and pending&accepted&declined shares
Accepted&declined shares table persist the same preference, but I suppose that is fine for now

Shared with others:

Sorting by name works
Sorting by Share receiver is broken
Sorting by Sharedate is broken

Shared via Link

Sorting by name works
Sorting by Share receiver is broken
Sorting by Sharedate is broken

kulmann · 2022-01-21T07:59:07Z

Items per page seems to mostly work fine, just produced a This site appears to use a scroll-linked positioning effect. This may not work well with asynchronous panning; see https://firefox-source-docs.mozilla.org/performance/scroll-linked_effects.html for further details and to join the discussion on related tools and features! for me once in Firefox

Is that related to this PR? 🤔

Shared with me:

[Vue warn]: Property or method "targetRoute" is not defined on the instance but referenced during render. upon page navigation with only pending shares, pending&accepted and pending&accepted&declined shares

That's on master, but I fixed it now. The pending shares table was referencing a targetRoute variable that didn't exist anymore - just deleted it, because you can't click on resources in the pending shares table anyway, so there is no need for a target route. Additionally fixed, that the declined shares table had are-resources-clickable set to true, which doesn't make sense either, so I set are-resources-clickable to true now only for accepted shares, otherwise it's false.

Accepted&declined shares table persist the same preference, but I suppose that is fine for now

Yes, unfortunately the way the table is built this is not that easily changed, so I'd like to keep it for now. The better UX would be, that accepted and declined shares have their own sorting. Would like to look into that separately.

Shared with others:

Sorting by name works

Sorting by Share receiver is broken

Sorting by Sharedate is broken

Sorting by Shared with fixed - this was again a bug from current master. The fix was to handle undefined values during sorting by interpreting them as empty strings for sorting (undefined.toString() explodes, obviously).
Sorting by sharedate was not broken, but needed a page reload after clicking the broken Shared with sort button.
Same for Shared via Link

ownclouders · 2022-01-22T13:54:29Z

Results for oC10SharingExternal https://drone.owncloud.com/owncloud/web/22095/40/1
The following scenarios passed on retry:

webUISharingExternal/federationSharing.feature:106

ownclouders · 2022-01-22T13:57:16Z

Results for oC10iPhone3 https://drone.owncloud.com/owncloud/web/22095/49/1
The following scenarios passed on retry:

webUISharingPublicDifferentRoles/shareByPublicLinkDifferentRoles.feature:33

ownclouders · 2022-01-22T13:57:52Z

Results for oC10XGAPortrait3 https://drone.owncloud.com/owncloud/web/22095/45/1
The following scenarios passed on retry:

webUISharingPublicDifferentRoles/shareByPublicLinkDifferentRoles.feature:35
webUISharingPublicDifferentRoles/shareByPublicLinkDifferentRoles.feature:33

ownclouders · 2022-01-22T13:58:41Z

Results for oC10iPhone1 https://drone.owncloud.com/owncloud/web/22095/47/1
The following scenarios passed on retry:

webUIPrivateLinks/accessingPrivateLinks.feature:9

It's being used to auto-wire RouteQuery params to local storage. It also enforced RouteQuery params to be set in the URL, even if the route was visited without them. With this we start to always have a full state in the URL. Started with the usePagination composable which now handles the items per page internally, including persistence to localStorage. Later on this can be replaced with another storage (like settings in the backend).

ownclouders · 2022-01-22T14:48:15Z

Results for oC10SharingExternal https://drone.owncloud.com/owncloud/web/22099/40/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

ownclouders · 2022-01-22T15:29:43Z

Results for oC10SharingPermission https://drone.owncloud.com/owncloud/web/22100/35/1
The following scenarios passed on retry:

webUISharingPermissionsUsers/sharePermissionsUsers.feature:21

ownclouders · 2022-01-22T15:33:10Z

Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/22100/53/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

sonarqubecloud · 2022-01-22T15:50:07Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells

67.4% Coverage
0.9% Duplication

kulmann · 2022-01-23T10:50:00Z

Force merged without waiting for another CI run - the last one was green and the two commits didn't change anything that could explode in tests.

kulmann requested review from fschade and pascalwengerter as code owners January 20, 2022 12:48

kulmann self-assigned this Jan 20, 2022

kulmann force-pushed the persist-sorting-per-view branch from 884654a to 97ce203 Compare January 20, 2022 13:32

dschmidt approved these changes Jan 20, 2022

View reviewed changes

packages/web-app-files/src/composables/sort/useSort.ts Show resolved Hide resolved

dschmidt reviewed Jan 20, 2022

View reviewed changes

packages/web-app-files/src/composables/pagination/usePagination.ts Outdated Show resolved Hide resolved

dschmidt reviewed Jan 20, 2022

View reviewed changes

packages/web-pkg/src/composables/localStorage/useLocalStorage.ts Outdated Show resolved Hide resolved

dschmidt reviewed Jan 20, 2022

View reviewed changes

packages/web-pkg/src/composables/router/useRouteQueryPersisted.ts Outdated Show resolved Hide resolved

pascalwengerter requested changes Jan 20, 2022

View reviewed changes

dschmidt mentioned this pull request Jan 20, 2022

Add URL handling to markdown-editor (fixes #5928) #6134

Merged

10 tasks

fschade force-pushed the persist-sorting-per-view branch from 2adbde6 to bd89542 Compare January 22, 2022 13:17

kulmann and others added 9 commits January 22, 2022 15:14

Rename composables to a consistent structure

93a82df

Persist sort choices per view

c4afb14

refactor composables into more appropriate locations

51734ef

Fix sorting on shared with me page

9fd86ca

Address review comments

c89e56b

Fix sorting for undefined values

7734454

Add changelog entry

349c40d

fix failing tests

d0b2b84

fschade force-pushed the persist-sorting-per-view branch from bd89542 to d0b2b84 Compare January 22, 2022 14:16

dschmidt approved these changes Jan 22, 2022

View reviewed changes

kulmann added 2 commits January 23, 2022 11:37

Fix types in usePagination

fb418f2

Fix code smells

71cf274

kulmann merged commit 7f9b6c9 into master Jan 23, 2022

delete-merged-branch bot deleted the persist-sorting-per-view branch January 23, 2022 10:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist sorting per view #6290

Persist sorting per view #6290

kulmann commented Jan 20, 2022 •

edited

Loading

update-docs bot commented Jan 20, 2022

kulmann commented Jan 20, 2022

dschmidt left a comment

pascalwengerter left a comment

kulmann commented Jan 21, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

sonarqubecloud bot commented Jan 22, 2022

kulmann commented Jan 23, 2022

Persist sorting per view #6290

Persist sorting per view #6290

Conversation

kulmann commented Jan 20, 2022 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Open tasks:

update-docs bot commented Jan 20, 2022

kulmann commented Jan 20, 2022

dschmidt left a comment

Choose a reason for hiding this comment

pascalwengerter left a comment

Choose a reason for hiding this comment

kulmann commented Jan 21, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

ownclouders commented Jan 22, 2022

sonarqubecloud bot commented Jan 22, 2022

kulmann commented Jan 23, 2022

kulmann commented Jan 20, 2022 •

edited

Loading