Use inject/provide to define popover/dropdown container

[vinicius73]

It helps to define the container of all popovers in the component tree at the same time.

Solving part of

• #3154
• https://github.com/nextcloud/nextcloud-vue/issues/3300
• https://github.com/nextcloud/text/issues/2842
• https://github.com/nextcloud/server/issues/34404

<template>
<div :id="randomID">
    <!-- any popover (NcActions, NcEmojiPicker, ...)  will use the randomID as container by default -->
</div>
</template>

<script>
import { defineComponent } from 'vue'
import { NC_KEY_POPOVER_CONTAINER } from '@nextcloud/vue'

const randomHash = () => (Math.ceil((Math.random() * 100000) + 1000)).toString(16)

export default defineComponent({
  data() {
    return {
       randomID: `custom-container-${randomHash()}`,
    }
  },
  
  provide () {
    return {
       [NC_KEY_POPOVER_CONTAINER]: `#${this.randomID}`,
    }
  }
})
</script>

[raimund-schluessler]

@vinicius73 Could you please explain why changing the default container from body to some random div solves the issues mentioned? And it requires manual work from every app developer, because otherwise, it falls back to body anyways. This does not seem ideal.

[vinicius73]

@raimund-schluessler this problem appears when we have more than one instance of focus trap active ate the same time (as mentioned https://github.com/nextcloud/nextcloud-vue/issues/3154 and https://github.com/focus-trap/focus-trap/issues/428)

In text it happens when text is open in the viwer (a NcModal) and an option from menu bar is open (https://github.com/nextcloud/text/issues/2842)

The solution was moving the container of NcMenu (who uses NcPopover under the hood) to MenuBar element (https://github.com/nextcloud/text/pull/3066).

If the focus trap is created inside a children element of other focus trap, we do not have this error.

We've faced the same problem in server (https://github.com/nextcloud/nextcloud-vue/issues/3300) If any viewer is open, interact with sidebar (specially in share options) will trigger the same error.

The viewer app already handles part of this problem

https://github.com/nextcloud/viewer/blob/2b5ed86ec105f65b8e3990373885f59dc9772749/src/views/Viewer.vue#L906-L913

Catching the sidebar event, and update its focus trap instance.

So, using a common container solve the problem without hard changes in next-cloud components.

---

This PR does not change any actual behavior of NcPopover.

In some cases, we have a lot of NcActions or similar at the same context.

This change makes the app developers able to handle this problem (if necessary) without hard changes, until we work in a more robust solution (a singleton instance of `focus trap https://github.com/focus-trap/focus-trap/issues/428)

I've already prepared a PR for server

diff --git a/apps/files_sharing/src/views/SharingTab.provider.js b/apps/files_sharing/src/views/SharingTab.provider.js
new file mode 100644
index 00000000000..38fe212e57b
--- /dev/null
+++ b/apps/files_sharing/src/views/SharingTab.provider.js
@@ -0,0 +1,12 @@
+export const SHARING_TAB_ID = Symbol('menu::id')
+
+export const useSharingTabIDMixin = {
+	inject: {
+		$sharingTabID: { from: SHARING_TAB_ID, default: null },
+	},
+	computed: {
+		sharingTabIDSelector() {
+			return `#${this.$sharingTabID}`
+		},
+	},
+}
diff --git a/apps/files_sharing/src/views/SharingTab.vue b/apps/files_sharing/src/views/SharingTab.vue
index d1416a6c0d1..a5eea691ee4 100644
--- a/apps/files_sharing/src/views/SharingTab.vue
+++ b/apps/files_sharing/src/views/SharingTab.vue
@@ -21,7 +21,7 @@
   -->
 
 <template>
-	<div :class="{ 'icon-loading': loading }">
+	<div :id="randomID" :class="{ 'icon-loading': loading }">
 		<!-- error message -->
 		<div v-if="error" class="emptycontent" :class="{ emptyContentWithSections: sections.length > 0 }">
 			<div class="icon icon-error" />
@@ -89,6 +89,7 @@
 import { CollectionList } from 'nextcloud-vue-collections'
 import { generateOcsUrl } from '@nextcloud/router'
 import NcAvatar from '@nextcloud/vue/dist/Components/NcAvatar'
+import { NC_KEY_POPOVER_CONTAINER } from '@nextcloud/vue/dist/providers.js'
 import axios from '@nextcloud/axios'
 import { loadState } from '@nextcloud/initial-state'
 
@@ -120,8 +121,16 @@ export default {
 
 	mixins: [ShareTypes],
 
+	provide() {
+		return {
+			[NC_KEY_POPOVER_CONTAINER]: `#${this.randomID}`
+		}
+	},
+
 	data() {
 		return {
+			randomID: `SharingTab-${(Math.ceil((Math.random() * 900000) + 100)).toString(16)}`,
+
 			config: new Config(),
 
 			error: '',

[raimund-schluessler]

I see, but I am still a bit puzzled, since they had a common container already, it was just not a provided div, but simply body, no?. So what I don't get is, why switching away from body solves this.

Edit: I guess, I don't understand the solution completely yet :see_no_evil:

[max-nextcloud]

I see, but I am still a bit puzzled, since they had a common container already, it was just not a provided div, but simply body, no?. So what I don't get is, why switching away from body solves this.

My understanding is that if you have two focus traps sitting next to each other (for example as siblings) in the DOM they will compete for the focus. The focus can only be in one of the traps. But if one focus trap is the parent of the other the focus can be in the child and therefore in both focus traps - so they are 'happy'.

If dropdown menus are attached to the body they are outside of any possible other focus trap (in particular the modal) and we have the competing situation. Attaching them to an element inside the current focus trap will make sure the focus can be in both traps at the same time.

So the problem could be solved by changing the container of all popovers inside a modal. However this can be tedious if there are multiple menus. This commit allows changing the default container for all popovers in the modal at once.

[raimund-schluessler]

Ah, ok, I think now I got it. Thanks. I somehow was under the impression that it's about multiple NcActions fighting about the focus, for which it wouldn't help to give them a common container. But it's in fact about a NcModal and multiple NcActions within and giving the actions a container within the modal solves the issue.

So this PR just adds a convenience function for an app-specific solution that would be possible already (e.g. by touching every NcActions in a modal), right?

That's fine with me. But since it still requires devs to adjust their code, a note about this in the documentation for every component using the focus-trap (i.e. NcActions, NcModal, etc.) would be great.