Skip to content

Commit 9ee0a71

Browse files
Chartman123Chartman123
authored
Merge pull request #3432 from nextcloud/feat/remember-result-view
feat: remember last results view in localStorage
2 parents f8ad4ea + 8b6fcea commit 9ee0a71

3 files changed

Lines changed: 294 additions & 8 deletions

File tree

‎playwright/e2e/results-view.spec.ts‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ test.describe('Results view', () => {
5555
// from submit → results after submission causes a brief redirect loop,
5656
// so we use direct navigation instead of clicking the TopBar.
5757
await page.goto(page.url().replace(/\/submit.*$/, '/results'))
58-
await page.waitForURL(/\/results$/)
58+
await page.waitForURL(/\/results(?:\?.*)?$/)
5959
})
6060

6161
test('Summary tab shows submitted data', async ({ resultsView }) => {
@@ -80,20 +80,76 @@ test.describe('Results view', () => {
8080
// Should show the individual submission with the answers
8181
await expect(resultsView.responsesTab).toBeChecked()
8282
await expect(resultsView.responseCount).toBeVisible()
83+
await expect(resultsView.page).toHaveURL(/\/results\?view=responses$/)
8384
})
8485

85-
test('Tab switching between Summary and Responses', async ({ resultsView }) => {
86+
test('Tab switching between Summary and Responses updates the URL', async ({
87+
resultsView,
88+
}) => {
8689
// Start on Summary
8790
await expect(resultsView.summaryTab).toBeChecked()
91+
await expect(resultsView.page).toHaveURL(/\/results\?view=summary$/)
8892

8993
// Switch to Responses
9094
await resultsView.switchToResponses()
9195
await expect(resultsView.responsesTab).toBeChecked()
9296
await expect(resultsView.summaryTab).not.toBeChecked()
97+
await expect(resultsView.page).toHaveURL(/\/results\?view=responses$/)
9398

9499
// Switch back to Summary
95100
await resultsView.switchToSummary()
96101
await expect(resultsView.summaryTab).toBeChecked()
97102
await expect(resultsView.responsesTab).not.toBeChecked()
103+
await expect(resultsView.page).toHaveURL(/\/results\?view=summary$/)
104+
})
105+
106+
test('Explicit query route wins over remembered localStorage view', async ({
107+
page,
108+
resultsView,
109+
}) => {
110+
await page.evaluate(() => {
111+
const match = window.location.pathname.match(
112+
/\/apps\/forms\/([^/]+)\/results$/,
113+
)
114+
if (!match) {
115+
throw new Error('Expected results route before setting localStorage')
116+
}
117+
118+
localStorage.setItem(
119+
`nextcloud_forms_${match[1]}_activeResponseView`,
120+
'responses',
121+
)
122+
})
123+
124+
await page.goto(page.url().replace(/\/results.*$/, '/results?view=summary'))
125+
await page.waitForURL(/\/results\?view=summary$/)
126+
127+
await expect(resultsView.summaryTab).toBeChecked()
128+
await expect(resultsView.responsesTab).not.toBeChecked()
129+
})
130+
131+
test('Query-less results route restores the remembered localStorage view', async ({
132+
page,
133+
resultsView,
134+
}) => {
135+
await page.evaluate(() => {
136+
const match = window.location.pathname.match(
137+
/\/apps\/forms\/([^/]+)\/results$/,
138+
)
139+
if (!match) {
140+
throw new Error('Expected results route before setting localStorage')
141+
}
142+
143+
localStorage.setItem(
144+
`nextcloud_forms_${match[1]}_activeResponseView`,
145+
'responses',
146+
)
147+
})
148+
149+
await page.goto(page.url().replace(/\/results.*$/, '/results'))
150+
await page.waitForURL(/\/results\?view=responses$/)
151+
152+
await expect(resultsView.responsesTab).toBeChecked()
153+
await expect(resultsView.summaryTab).not.toBeChecked()
98154
})
99155
})

‎src/Forms.vue‎

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,53 @@ export default {
351351
loading.value = false
352352
}
353353
354+
/**
355+
* Clean up stale localStorage entries for forms that are no longer available.
356+
* Removes localStorage keys matching the pattern `nextcloud_forms_*_activeResponseView`
357+
* where the form hash no longer exists in the current forms list.
358+
*/
359+
const cleanupStaleLocalStorageEntries = () => {
360+
try {
361+
// Get all current form hashes
362+
const currentFormHashes = new Set(
363+
[...forms.value, ...allSharedForms.value].map(
364+
(form) => form.hash,
365+
),
366+
)
367+
368+
// Iterate through all localStorage keys
369+
const keysToRemove = []
370+
for (let i = 0; i < localStorage.length; i++) {
371+
const key = localStorage.key(i)
372+
if (
373+
key
374+
&& key.startsWith('nextcloud_forms_')
375+
&& key.endsWith('_activeResponseView')
376+
) {
377+
// Extract hash from key: nextcloud_forms_<hash>_activeResponseView
378+
const hash = key.substring(
379+
'nextcloud_forms_'.length,
380+
key.length - '_activeResponseView'.length,
381+
)
382+
// If form hash is not in current forms, mark for removal
383+
if (!currentFormHashes.has(hash)) {
384+
keysToRemove.push(key)
385+
}
386+
}
387+
}
388+
389+
// Remove stale entries
390+
keysToRemove.forEach((key) => {
391+
localStorage.removeItem(key)
392+
logger.debug(`Removed stale localStorage entry: ${key}`)
393+
})
394+
} catch (err) {
395+
logger.debug('Error cleaning up stale localStorage entries', {
396+
error: err,
397+
})
398+
}
399+
}
400+
354401
/**
355402
* Fetch a partial form by its hash after initial load completes.
356403
*
@@ -447,6 +494,17 @@ export default {
447494
forms.value.splice(formIndex, 1)
448495
deletedFormHash.value = deletedHash
449496
497+
// Remove localStorage entry for this form's active response view
498+
try {
499+
localStorage.removeItem(
500+
`nextcloud_forms_${deletedHash}_activeResponseView`,
501+
)
502+
} catch (err) {
503+
logger.debug('Error removing localStorage entry for deleted form', {
504+
error: err,
505+
})
506+
}
507+
450508
if (deletedHash === routeHash.value && route.name !== 'root') {
451509
// Navigate to root without triggering route guards
452510
router.replace({ name: 'root' })
@@ -477,8 +535,9 @@ export default {
477535
}
478536
}
479537
480-
onMounted(() => {
481-
loadForms()
538+
onMounted(async () => {
539+
await loadForms()
540+
cleanupStaleLocalStorageEntries()
482541
subscribe('forms:last-updated:set', onLastUpdatedByEventBus)
483542
subscribe('forms:ownership-transfered', onDeleteForm)
484543
})

‎src/views/Results.vue‎

Lines changed: 175 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
:options="responseViews"
4848
:groupLabel="t('forms', 'View mode')"
4949
class="response-actions__toggle"
50-
@update:active="loadFormResults" />
50+
@update:active="onChangeResponseView" />
5151

5252
<!-- Action menu for cloud export and deletion -->
5353
<NcActions
@@ -328,6 +328,7 @@ const responseViews = [
328328
id: 'responses',
329329
},
330330
]
331+
const responseViewIds = new Set(responseViews.map((view) => view.id))
331332
332333
export default {
333334
// eslint-disable-next-line vue/multi-word-component-names
@@ -379,7 +380,7 @@ export default {
379380
380381
data() {
381382
return {
382-
activeResponseView: responseViews[0],
383+
activeResponseView: null,
383384
384385
questions: [],
385386
submissions: [],
@@ -498,10 +499,16 @@ export default {
498499
// Reload results when form changes
499500
async hash() {
500501
await this.fetchFullForm(this.form.id)
501-
this.loadFormResults()
502+
await this.syncActiveResponseViewFromRoute()
502503
SetWindowTitle(this.formTitle)
503504
},
504505
506+
'$route.query': {
507+
handler() {
508+
this.syncActiveResponseViewFromRoute()
509+
},
510+
},
511+
505512
limit() {
506513
this.loadFormResults()
507514
},
@@ -521,15 +528,179 @@ export default {
521528
})
522529
this.loadFormResults()
523530
}, INPUT_DEBOUNCE_MS),
531+
532+
// Persist active response view to localStorage when it changes
533+
activeResponseView(newView) {
534+
if (newView?.id) {
535+
this.saveActiveResponseViewToLocalStorage(newView.id)
536+
}
537+
},
524538
},
525539
526540
async beforeMount() {
527541
await this.fetchFullForm(this.form.id)
528-
this.loadFormResults()
542+
await this.syncActiveResponseViewFromRoute()
529543
SetWindowTitle(this.formTitle)
530544
},
531545
532546
methods: {
547+
/**
548+
* Resolve a response view object by its ID.
549+
*
550+
* @param {string} viewId The requested response view ID
551+
* @return {object}
552+
*/
553+
getResponseViewById(viewId) {
554+
return (
555+
responseViews.find((view) => view.id === viewId) ?? responseViews[0]
556+
)
557+
},
558+
559+
/**
560+
* Read the explicit response view from the current route query.
561+
*
562+
* @return {string|null}
563+
*/
564+
getRouteResponseViewId() {
565+
return this.$route.query.view ?? null
566+
},
567+
568+
/**
569+
* Load the stored response view preference from localStorage for the current form.
570+
*
571+
* @return {string}
572+
*/
573+
loadStoredActiveResponseViewId() {
574+
try {
575+
const storageKey = this.getActiveResponseViewStorageKey()
576+
if (!storageKey) {
577+
return responseViews[0].id
578+
}
579+
580+
const storedViewId = localStorage.getItem(storageKey)
581+
if (storedViewId && responseViewIds.has(storedViewId)) {
582+
return storedViewId
583+
}
584+
585+
return responseViews[0].id
586+
} catch (err) {
587+
logger.debug('Error loading activeResponseView from localStorage', {
588+
error: err,
589+
})
590+
return responseViews[0].id
591+
}
592+
},
593+
594+
/**
595+
* Resolve the effective response view using route state first and localStorage second.
596+
*
597+
* @return {string}
598+
*/
599+
resolveActiveResponseViewId() {
600+
return (
601+
this.getRouteResponseViewId()
602+
?? this.loadStoredActiveResponseViewId()
603+
)
604+
},
605+
606+
/**
607+
* Apply the effective route/localStorage view and refresh results when needed.
608+
*/
609+
async syncActiveResponseViewFromRoute() {
610+
const routeViewId = this.getRouteResponseViewId()
611+
const nextView = this.getResponseViewById(
612+
routeViewId ?? this.loadStoredActiveResponseViewId(),
613+
)
614+
const currentViewId = this.activeResponseView?.id
615+
616+
if (currentViewId !== nextView.id) {
617+
this.activeResponseView = nextView
618+
}
619+
620+
if (!routeViewId) {
621+
try {
622+
await this.$router.replace({
623+
name: 'results',
624+
params: {
625+
hash: this.form.hash,
626+
},
627+
query: {
628+
...this.$route.query,
629+
view: nextView.id,
630+
},
631+
})
632+
return
633+
} catch (error) {
634+
logger.debug('Navigation cancelled', { error })
635+
}
636+
}
637+
638+
this.loadFormResults()
639+
},
640+
641+
/**
642+
* Save the active response view preference to localStorage for the current form.
643+
*
644+
* @param {string} viewId - The ID of the view ('summary' or 'responses')
645+
*/
646+
saveActiveResponseViewToLocalStorage(viewId) {
647+
try {
648+
const storageKey = this.getActiveResponseViewStorageKey()
649+
if (!storageKey) {
650+
return
651+
}
652+
653+
localStorage.setItem(storageKey, viewId)
654+
} catch (err) {
655+
logger.debug('Error saving activeResponseView to localStorage', {
656+
error: err,
657+
})
658+
}
659+
},
660+
661+
/**
662+
* Build the localStorage key for the active response view.
663+
*
664+
* @return {string|null}
665+
*/
666+
getActiveResponseViewStorageKey() {
667+
const formHash = this.form?.hash
668+
if (!formHash) {
669+
return null
670+
}
671+
672+
return `nextcloud_forms_${formHash}_activeResponseView`
673+
},
674+
675+
/**
676+
* Navigate to an explicit route query for the selected response view.
677+
*
678+
* @param {object} view The selected response view object
679+
*/
680+
async onChangeResponseView(view) {
681+
if (!view?.id) {
682+
return
683+
}
684+
if (this.getRouteResponseViewId() === view.id) {
685+
this.loadFormResults()
686+
return
687+
}
688+
689+
try {
690+
await this.$router.push({
691+
name: 'results',
692+
params: {
693+
hash: this.form.hash,
694+
},
695+
query: {
696+
view: view.id,
697+
},
698+
})
699+
} catch (error) {
700+
logger.debug('Navigation cancelled', { error })
701+
}
702+
},
703+
533704
async onUnlinkFile() {
534705
await axios.patch(
535706
generateOcsUrl('apps/forms/api/v3/forms/{formId}', {

0 commit comments

Comments
 (0)