[2.6] Actions menu misplaced, scrolling not possible

[raimund-schluessler]

With the adjusted actions dropdown, the positioning is wrong when you start to scroll (far). And what makes this issue worse, is that the content jumps to the position of the popover when you open the menu while scrolled down. This means it is impossible to ever see the actions menu popover while at the bottom of the page. See here:

Also, in some cases an x-y-scroll bar appears when opening the menu. But this is just an annoyance and no critical usability issue:

@skjnldsv and @ma12-co (since I think you worked on the actions popover).

[marcoambrosini]

So I presume that this problem arises because the container of the popover is hard coded to be the body

https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/Actions/Actions.vue#L121

I think that by appending it to the parent element (the toggle), would solve this issue . We should keep the body as boundaries element though.
What do you think @skjnldsv ??

[skjnldsv]

It needs testing. No idea here

[raimund-schluessler]

So I presume that this problem arises because the container of the popover is hard coded to be the body

https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/Actions/Actions.vue#L121

I think that by appending it to the parent element (the toggle), would solve this issue . We should keep the body as boundaries element though.

I guess this would fix this particular issue. But I would be careful to not reintroduce things like this:

[raimund-schluessler]

How about adding a prop for container, so the dev can decide where it should be attached? We keep the default as body, so nothing breaks, but we can react to cases like the one above.

I think the actual problem is, that v-tooltip has issues when the element is position: fixed. I will check and create an upstream issue if applicable.

[raimund-schluessler]

Apparently, only setting the container to the parent element (the toggle) or to the position: fixed header does not fix the issue. It seems that while scrolling down, the body moves up and the popover tries to stay within the body. Hence, the popover is visible when scrolled down. See here:

[raimund-schluessler]

Ok, so the main problem is the boundaries-element. Since it was body by default, and body scrolls out of the viewport, the popover would scroll out of the viewport as well. Setting the boundaries-element to document.querySelector('.content.app-tasks') with the available prop helped a lot already:

However, the popover would still flutter while scrolling and the viewport would jump to the top when opening the popover while scrolled down.

With #1389 and setting the container to .header, both remaining issues are solved. The popover is nicely static while scrolling and no jumping occurs:

[raimund-schluessler]

While looking at this, I just realized the status select popover has the same problem:

I created an issue in server nextcloud/server#22606.

[raimund-schluessler]

Thinking about it, it might be better to not use body as the boundaries element by default, because it will always create issues when scrolling.

[raimund-schluessler]

Thinking about it, it might be better to not use body as the boundaries element by default, because it will always create issues when scrolling.

This is indeed more critical than I thought. Using body as boundariesElement breaks all actions which are only visible after you scroll. And it moves all other actions out of the viewport after scrolling far enough. So body is definitly no good default element for the boundaries.

[raimund-schluessler]

document.querySelector('#content') or document.querySelector('#content-vue') would be an option for a better boundaries element. But the problem is that #content exists already on page load and is replaced with #content-vue. So any action which is bound on page load to #content (e.g. the status selection in the header) will not work anymore once #content-vue is created.

So I don't see how we get the status select popover to work properly for vue apps.

I think the solution is to set

html, body {
	min-height: 100%;
}

for the server https://github.com/nextcloud/server/blob/master/core/css/styles.scss#L31-L33 instead of height: 100%;. After a quick test this seems to fix the issue and not break anything. But since this is a change which would effect everything on Nextcloud, this would need to be checked very well.

#1389 is still necessary to prevent the fluttering of the actions popover, but that's only a small annoyance.

[raimund-schluessler]

I created a PR in the server repo nextcloud/server#22614. Please test this thoroughly.

[raimund-schluessler]

Could we get a bit of feedback here? I think this is really an important issue, because it affects every popover / action menu for every vue app and additionally every vue popover in server. While scrolled down none of them opens and they move out of the viewport when scrolling. @nextcloud/designers @nextcloud/vuejs

[skjnldsv]

I'm personally cold feet towards big late css changes like those.
But we could do it, it's an easy rollback

[raimund-schluessler]

I'm personally cold feet towards big late css changes like those.

I am also not very happy about that. But I think leaving the actions popovers broken is not an option either.

If someone finds another solution without CSS changes for the server, I am all in.

[raimund-schluessler]

Not really fixed as long as nextcloud/server#22614 is not merged.

[raimund-schluessler]

Fixed in server now.

[raimund-schluessler]

How should we handle this for older server versions than 20? When you update nextcloud-vue for your app, the Actions will only work by default on NC20, but break on older server versions (as long as you don't manually select a proper boundaries-element).

We either have to advise the devs to adjust the boundaries-element to something like #content-vue, backport nextcloud/server#22614 to older server versions or only support NC20.

[raimund-schluessler]

Since nextcloud/server#22614 creates trouble, we probably have to revert it nextcloud/server#22773. So I reopen this here.

[raimund-schluessler]

I tried to find another solution than setting min-height: 100% like using other boundary elements such as #content, #content-vue or even undefined, but none of them works properly. From my side this will remain broken.

I will adjust all Actions in the Tasks app to use #content-vue, so the Actions will work at least within the app, but this will need to be done for every app using nextcloud-vue Actions and will not work to fix the status select popover.

[raimund-schluessler]

I will adjust all Actions in the Tasks app to use #content-vue, so the Actions will work at least within the app, but this will need to be done for every app using nextcloud-vue Actions and will not work to fix the status select popover.

Sadly, I have to revert this statement. It is not possible to fix all Actions components from within an app, since some Actions are used by the vue-components directly, e.g. in the AppNavigationItem component https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/AppNavigationItem/AppNavigationItem.vue#L148-L159. There is no possibility to change the boundaries-element from the app itself, as the prop is not exposed. Hence, one would need to change the default boundaries-element for the Actions from body to e.g. #content-vue to fix it for all vue apps, but this would break server when Actions are used outside a vue app.

I know bugs can happen, as I have introduced enough bugs for multiple releases of vue-components lately myself 🙈 , but deciding to not fix such an impacting problem until the next major server release with Nextcloud 21 is not satisfying. This practically means that there will be no new release of Tasks for NC20, as every new release will have broken Actions menues which I see as a major regression. I will continue to work on issues and features for Tasks, as I am sure this will get fixed at some point, but I will have to postpone any new release until after a fix, to not introduce a regression.

I am happy to hear any workaround or real solution for the problems described, I just haven't found any myself so far.

[raimund-schluessler]

Ok, I don't know why I didn't realize this earlier, but I can just add these lines to the app for which the Actions are broken, and it's fixed:

body {
	min-height: 100%;
	height: auto;
}

This doesn't break anything for the rest of Nextcloud and fixes the problem for now. I will create a PR for Tasks and I guess every app which also has this problem can just add this for now. For sure not ideal, but it works. Sorry for the fuzz. 🙈

[onny]

Ok, I don't know why I didn't realize this earlier, but I can just add these lines to the app for which the Actions are broken, and it's fixed:

body {
	min-height: 100%;
	height: auto;
}

This doesn't break anything for the rest of Nextcloud and fixes the problem for now. I will create a PR for Tasks and I guess every app which also has this problem can just add this for now. For sure not ideal, but it works. Sorry for the fuzz. see_no_evil

Thank you so much for your work! This fixed the issue for me for the rewrite of the Radio app.

[skjnldsv]

@onny if you're on latest master or 20, this is already into server, if you use the proper templates to render you page, it should not require you to have this into the radio app

[skjnldsv]

I guess we can close then

[raimund-schluessler]

@skjnldsv This is not in server yet, neither master https://github.com/nextcloud/server/blob/master/core/css/styles.scss#L17-L41 nor 20. We had to revert the respective PR, since it caused issues with Talk.

[raimund-schluessler]

I guess now would be the right time to indeed fix it in master and adjust Talk.

[skjnldsv]

I guess now would be the right time to indeed fix it in master and adjust Talk.

Ah sorry Raimund, I forgot! Thanks for reminding me :)
Enjoy the weekend! 🌟

[susnux]

Can we fix this for the NC27 circle?

[skjnldsv]

Can we fix this for the NC27 circle?

What are you referring to?

[susnux]

Well, I stumbled over comments in some apps referring to this issue, and as I did not test if those workarounds are still needed, and from the discussion above, I thought this issue is still affecting apps.
But after testing at least the forms and tasks app without those workarounds, it seems this issue is already fixed and the workarounds are no longer required.

So maybe this issue can be closed.