fix(NcAppSettingsDialog): unregisterSection should remove the section instead of remove all other

src/components/NcAppSettingsDialog/NcAppSettingsDialog.vue

@@ -181,6 +181,7 @@ import { useIsMobile } from '../../composables/useIsMobile/index.js'
 import { t } from '../../l10n.js'
 
 import debounce from 'debounce'
+import Vue from 'vue'
 
 export default {
 
@@ -194,7 +195,7 @@ export default {
 	provide() {
 		return {
 			registerSection: this.registerSection,
-			unregisterSection: this.registerSection,
+			unregisterSection: this.unregisterSection,
 		}
 	},
 
@@ -256,7 +257,7 @@ export default {
 			scroller: null,
 			/**
 			 * Currently registered settings sections
-			 * @type {{ id: string, name: string, icon?: VNode[]  }}
+			 * @type {{ id: string, name: string, icon?: import('vue').VNode[] }[]}
 			 */
 			sections: [],
 		}
@@ -295,11 +296,6 @@ export default {
 		},
 	},
 
-	mounted() {
-		// Select first settings section
-		this.selectedSection = this.$slots.default[0].componentOptions.propsData.id
-	},
-
 	updated() {
 		// Check that the scroller element has been mounted
 		if (!this.$refs.settingsScroller) {
@@ -327,23 +323,33 @@ export default {
 				throw new Error(`Duplicate section id found: ${id}. Settings navigation sections must have unique section ids.`)
 			}
 			if (this.sections.some(({ name: otherName }) => name === otherName)) {
-				throw new Error(`Duplicate section name found: ${name}. Settings navigation sections must have unique section names.`)
+				Vue.util.warn(`Duplicate section name found: ${name}. Settings navigation sections must have unique section names.`)
 			}
 
 			const newSections = [...this.sections, { id, name, icon }]
 			// Sort sections by order in slots
 			this.sections = newSections.sort(({ id: idA }, { id: idB }) => {
-				const indexOf = (id) => this.$slots.default.indexOf(vnode => vnode?.componentOptions?.propsData?.id === id)
+				const indexOf = (id) => this.$slots.default?.findIndex?.(vnode => vnode?.componentOptions?.propsData?.id === id) ?? -1
 				return indexOf(idA) - indexOf(idB)
 			})
+
+			// If this is the first section registered, set it as selected
+			if (this.sections.length === 1) {
+				this.selectedSection = id
+			}
 		},
 
 		/**
-		 * Called when a new section is unregistered
+		 * Called when a section is unregistered to remove it from dialog
 		 * @param {string} id The section ID
 		 */
 		unregisterSection(id) {
-			this.sections = this.sections.filter(({ id: otherId }) => id === otherId)
+			this.sections = this.sections.filter(({ id: otherId }) => id !== otherId)
+
+			// If the current section is unregistered, set the first section as selected
+			if (this.selectedSection === id) {
+				this.selectedSection = this.sections[0]?.id ?? ''
+			}
 		},
 
 		/**

tests/unit/components/NcAppSettingsDialog/NcAppSettingsDialog.spec.ts

@@ -0,0 +1,131 @@
+/**
+ * @copyright Copyright (c) 2023 Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @author Ferdinand Thiessen <opensource@fthiessen.de>
+ *
+ * @license AGPL-3.0-or-later
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+import { mount } from '@vue/test-utils'
+import NcAppSettingsDialog from '../../../../src/components/NcAppSettingsDialog/NcAppSettingsDialog.vue'
+import { defineComponent, inject, nextTick, onMounted } from 'vue'
+
+/**
+ * Mocked AppSettingsSection that just registers it self
+ */
+const MockSection = defineComponent({
+	props: { id: { type: String, default: 'test_id' } },
+	setup(props) {
+		const register = inject<(id: string, name: string)=> void>('registerSection')
+		onMounted(() => register?.(props.id, 'test_name'))
+		return (h) => h('li', ['empty'])
+	},
+})
+
+describe('NcAppSettingsDialog: Sections registration', () => {
+	it('injects register function to children', async () => {
+		const wrapper = mount(NcAppSettingsDialog, {
+			slots: {
+				default: MockSection,
+			},
+			propsData: {
+				open: true,
+			},
+		})
+
+		await nextTick()
+
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(1)
+		expect(wrapper.find('[role="tab"]').text()).toBe('test_name')
+	})
+
+	it('can register a new section', async () => {
+		// eslint-disable-next-line @typescript-eslint/no-explicit-any
+		const wrapper = mount<Vue & { registerSection: any }>(NcAppSettingsDialog, {
+			propsData: {
+				open: true,
+			},
+		})
+
+		wrapper.vm.registerSection('test_id', 'test_name')
+		await nextTick()
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(1)
+		expect(wrapper.find('[role="tab"]').text()).toBe('test_name')
+	})
+
+	it('warn on register a already registered section name', async () => {
+		// eslint-disable-next-line @typescript-eslint/no-explicit-any
+		const wrapper = mount<Vue & { registerSection: any }>(NcAppSettingsDialog, {
+			propsData: {
+				open: true,
+			},
+		})
+
+		const spy = jest.spyOn(globalThis.console, 'error')
+		spy.mockImplementationOnce(() => {})
+
+		// First call should be OK
+		wrapper.vm.registerSection('test_id', 'test_name')
+		await nextTick()
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(1)
+		expect(spy).not.toHaveBeenCalled()
+
+		// Second one should unregister first and replace with this one, but show an error
+		wrapper.vm.registerSection('test_id_2', 'test_name')
+		await nextTick()
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(2)
+		expect(spy).toHaveBeenCalled()
+	})
+
+	it('error on register a already registered section ID', async () => {
+		// eslint-disable-next-line @typescript-eslint/no-explicit-any
+		const wrapper = mount<Vue & { registerSection: any }>(NcAppSettingsDialog, {
+			propsData: {
+				open: true,
+			},
+		})
+
+		// First call should be OK
+		wrapper.vm.registerSection('test_id', 'test_name')
+		await nextTick()
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(1)
+
+		// Second one should unregister first and replace with this one, but show an error
+		expect(() => wrapper.vm.registerSection('test_id', 'test_other_name')).toThrow()
+		await nextTick()
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(1)
+	})
+
+	it('can unregister a section', async () => {
+		// eslint-disable-next-line @typescript-eslint/no-explicit-any
+		const wrapper = mount<Vue & { registerSection: any, unregisterSection: any }>(NcAppSettingsDialog, {
+			propsData: {
+				open: true,
+			},
+		})
+
+		wrapper.vm.registerSection('test_id', 'test_name')
+		wrapper.vm.registerSection('test_id2', 'test_name2')
+		await nextTick()
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(2)
+
+		wrapper.vm.unregisterSection('test_id')
+		await nextTick()
+		expect(wrapper.findAll('[role="tab"]')).toHaveLength(1)
+		expect(wrapper.find('[role="tab"]').text()).toBe('test_name2')
+	})
+})