fix(NcAppSIdebarTabs): fix dynamic tabs registration

fix: #3461

Signed-off-by: Grigorii Shartsev <grigorii.shartsev@nextcloud.com>

Co-authored-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>

Author: ShGKme

src/components/NcAppSidebar/NcAppSidebar.vue

@@ -31,22 +31,33 @@ include a standard-header like it's used by the files app.
 
 ```vue
 <template>
-	<NcAppSidebar
-		title="cat-picture.jpg"
-		subtitle="last edited 3 weeks ago">
-		<NcAppSidebarTab name="Settings" id="settings-tab">
-			<template #icon>
-				<Cog :size="20" />
-			</template>
-			Settings tab content
-		</NcAppSidebarTab>
-		<NcAppSidebarTab name="Sharing" id="share-tab">
-			<template #icon>
-				<ShareVariant :size="20" />
-			</template>
-			Sharing tab content
-		</NcAppSidebarTab>
-	</NcAppSidebar>
+	<div>
+		<NcCheckboxRadioSwitch :checked.sync="hideFirstTab">
+			The first tab is hidden
+		</NcCheckboxRadioSwitch>
+		<NcAppSidebar
+			title="cat-picture.jpg"
+			subtitle="last edited 3 weeks ago">
+			<NcAppSidebarTab v-if="!hideFirstTab" name="Fist tab" id="first-tab">
+				<template #icon>
+					<Cog :size="20" />
+				</template>
+				New tab
+			</NcAppSidebarTab>
+			<NcAppSidebarTab name="Settings" id="settings-tab">
+				<template #icon>
+					<Cog :size="20" />
+				</template>
+				Settings tab content
+			</NcAppSidebarTab>
+			<NcAppSidebarTab name="Sharing" id="share-tab">
+				<template #icon>
+					<ShareVariant :size="20" />
+				</template>
+				Sharing tab content
+			</NcAppSidebarTab>
+		</NcAppSidebar>
+	</div>
 </template>
 <script>
 	import Cog from 'vue-material-design-icons/Cog'
@@ -57,10 +68,87 @@ include a standard-header like it's used by the files app.
 			Cog,
 			ShareVariant,
 		},
+		data() {
+			return {
+				hideFirstTab: true,
+			}
+		},
 	}
 </script>
 ```
 
+### One tab
+
+```vue
+<template>
+	<div>
+		<NcAppSidebar
+			title="cat-picture.jpg"
+			subtitle="last edited 3 weeks ago">
+			<NcAppSidebarTab name="Settings" id="settings-tab">
+				<template #icon>
+					<Cog :size="20" />
+				</template>
+				New tab
+			</NcAppSidebarTab>
+		</NcAppSidebar>
+	</div>
+</template>
+<script>
+import Cog from 'vue-material-design-icons/Cog'
+
+export default {
+	components: {
+		Cog,
+	},
+}
+</script>
+```
+
+### One or two tabs with condition
+
+```vue
+<template>
+	<div>
+		<NcCheckboxRadioSwitch :checked.sync="hideFirstTab">
+			The first tab is hidden
+		</NcCheckboxRadioSwitch>
+		<NcAppSidebar
+			title="cat-picture.jpg"
+			subtitle="last edited 3 weeks ago">
+			<NcAppSidebarTab v-if="!hideFirstTab" name="Settings" id="settings-tab">
+				<template #icon>
+					<Cog :size="20" />
+				</template>
+				Settings
+			</NcAppSidebarTab>
+			<NcAppSidebarTab name="Sharing" id="share-tab">
+				<template #icon>
+					<ShareVariant :size="20" />
+				</template>
+				Sharing tab content
+			</NcAppSidebarTab>
+		</NcAppSidebar>
+	</div>
+</template>
+<script>
+import Cog from 'vue-material-design-icons/Cog'
+import ShareVariant from 'vue-material-design-icons/ShareVariant'
+
+export default {
+	components: {
+		Cog,
+		ShareVariant,
+	},
+	data() {
+		return {
+			hideFirstTab: true,
+		}
+	},
+}
+</script>
+```
+
 ### Editable title
 
 ```vue

src/components/NcAppSidebar/NcAppSidebarTabs.vue

@@ -47,8 +47,7 @@
 						role="tab"
 						@click.prevent="setActive(tab.id)">
 						<span class="app-sidebar-tabs__tab-icon">
-							<NcVNodes v-if="hasMdIcon(tab)" :vnodes="tab.$slots.icon[0]" />
-							<span v-else :class="tab.icon" />
+							<NcVNodes :vnodes="tab.renderIcon()" />
 						</span>
 						{{ tab.name }}
 					</a>
@@ -67,16 +66,6 @@
 <script>
 import NcVNodes from '../NcVNodes/index.js'
 
-import Vue from 'vue'
-
-const IsValidString = function(value) {
-	return value && typeof value === 'string' && value.trim() !== ''
-}
-
-const IsValidStringWithoutSpaces = function(value) {
-	return IsValidString(value) && value.indexOf(' ') === -1
-}
-
 export default {
 	name: 'NcAppSidebarTabs',
 
@@ -85,6 +74,15 @@ export default {
 		NcVNodes,
 	},
 
+	provide() {
+		return {
+			registerTab: this.registerTab,
+			unregisterTab: this.unregisterTab,
+			// Getter as an alternative to Vue 2.7 computed(() => this.activeTab)
+			getActiveTab: () => this.activeTab,
+		}
+	},
+
 	props: {
 		/**
 		 * Id of the tab to activate
@@ -107,10 +105,6 @@ export default {
 			 * The id of the currently active tab.
 			 */
 			activeTab: '',
-			/**
-			 * Dummy array to react on slot changes.
-			 */
-			children: [],
 		}
 	},
 
@@ -130,18 +124,6 @@ export default {
 				this.updateActive()
 			}
 		},
-
-		children() {
-			this.updateTabs()
-		},
-	},
-
-	mounted() {
-		// Init the tabs list
-		this.updateTabs()
-
-		// Let's make the children list reactive
-		this.children = this.$children
 	},
 
 	methods: {
@@ -216,62 +198,42 @@ export default {
 		 */
 		updateActive() {
 			this.activeTab = this.active
-				&& this.tabs.findIndex(tab => tab.id === this.active) !== -1
+			&& this.tabs.some(tab => tab.id === this.active)
 				? this.active
 				: this.tabs.length > 0
 					? this.tabs[0].id
 					: ''
 		},
 
-		hasMdIcon(tab) {
-			return tab?.$slots?.icon
-		},
-
 		/**
-		 * Manually update the sidebar tabs according to $slots.default
+		 * Register child tab in the tabs
+		 *
+		 * @param {object} tab - tab props (only the "id" is used actually)
 		 */
-		updateTabs() {
-			if (!this.$slots.default) {
-				this.tabs = []
-				return
-			}
-
-			// Find all valid children (AppSidebarTab, other components, text nodes, etc.)
-			const children = this.$slots.default.filter(elem => elem.tag || elem.text.trim())
-
-			// Find all valid instances of AppSidebarTab
-			const invalidTabs = []
-			const tabs = children.reduce((tabs, tabNode) => {
-				const tab = tabNode.componentInstance
-				// Make sure all required props are provided and valid
-				if (IsValidString(tab?.name)
-					&& IsValidStringWithoutSpaces(tab?.id)
-					&& (IsValidStringWithoutSpaces(tab?.icon) || tab?.$slots?.icon)) {
-					tabs.push(tab)
-				} else {
-					invalidTabs.push(tabNode)
-				}
-				return tabs
-			}, [])
-
-			// Tabs are optional, but you can use either tabs or non-tab-content only
-			if (tabs.length !== 0 && tabs.length !== children.length) {
-				Vue.util.warn('Mixing tabs and non-tab-content is not possible.')
-				invalidTabs.map(invalid => console.debug('Ignoring invalid tab', invalid))
-			}
-
-			// We sort the tabs by their order or by their name
-			this.tabs = tabs.sort((a, b) => {
-				const orderA = a.order || 0
-				const orderB = b.order || 0
-				if (orderA === orderB) {
+		registerTab(tab) {
+			this.tabs.push(tab)
+			this.tabs.sort((a, b) => {
+				if (a.order === b.order) {
 					return OC.Util.naturalSortCompare(a.name, b.name)
 				}
-				return orderA - orderB
+				return a.order - b.order
 			})
+			if (!this.activeTab) {
+				this.updateActive()
+			}
+		},
 
-			// Init active tab if exists
-			if (this.tabs.length > 0) {
+		/**
+		 * Unregister child tab in the tabs
+		 *
+		 * @param {string} id - tab's id
+		 */
+		unregisterTab(id) {
+			const tabIndex = this.tabs.findIndex((tab) => tab.id === id)
+			if (tabIndex !== -1) {
+				this.tabs.splice(tabIndex, 1)
+			}
+			if (this.activeTab === id) {
 				this.updateActive()
 			}
 		},

src/components/NcAppSidebarTab/NcAppSidebarTab.vue

@@ -40,9 +40,13 @@
 </template>
 
 <script>
+import { h } from 'vue'
+
 export default {
 	name: 'NcAppSidebarTab',
 
+	inject: ['registerTab', 'unregisterTab', 'getActiveTab'],
+
 	props: {
 		id: {
 			type: String,
@@ -67,13 +71,22 @@ export default {
 		'scroll',
 	],
 
+	expose: ['id', 'name', 'icon', 'order', 'renderIcon'],
+
 	computed: {
-		// TODO: implement a better way to force pass a prop fromm Sidebar
 		isActive() {
-			return this.$parent.activeTab === this.id
+			return this.getActiveTab() === this.id
 		},
 	},
 
+	created() {
+		this.registerTab(this)
+	},
+
+	beforeDestroy() {
+		this.unregisterTab(this.id)
+	},
+
 	methods: {
 		onScroll(event) {
 			// Are we scrolled to the very bottom ?
@@ -85,6 +98,15 @@ export default {
 			}
 			this.$emit('scroll', event)
 		},
+
+		/**
+		 * Render tab's icon from slot or icon prop
+		 *
+		 * @return {import('vue').VNode|import('vue').VNode[]}
+		 */
+		renderIcon() {
+			return this.$slots.icon || this.$scopedSlots.icon?.() || h('span', { staticClass: this.icon })
+		},
 	},
 }
 </script>

tests/unit/components/NcAppSidebar/NcAppSidebarTabs.spec.js

@@ -189,23 +189,4 @@ describe('NcAppSidebarTabs.vue', () => {
 			})
 		})
 	})
-	describe('when tabs and other elements are mixed', () => {
-		it('Issues a warning and logs to console .', () => {
-			mount(NcAppSidebarTabs, {
-				slots: {
-					default: [
-						'<nc-app-sidebar-tab id="1" icon="icon-details" name="Tab1">Tab1</nc-app-sidebar-tab>',
-						'<NcAppSidebarTab id="2" icon="icon-details" name="Tab2">Tab2</NcAppSidebarTab>',
-						'<div>Non-tab-content</div>',
-						'Test',
-					],
-				},
-				stubs: {
-					NcAppSidebarTab,
-				},
-			})
-			expect(onWarning).toHaveBeenCalledTimes(1)
-			expect(consoleDebug).toHaveBeenCalledTimes(2)
-		})
-	})
 })