Fix container of modal and popup components when Talk is embedded #6119
Conversation
|
/backport to stable22.1 |
|
/backport to stable21 |
The container of modal and popup components was always set to "#content-vue" to ensure that they will be properly shown in normal and fullscreen mode in the main Talk UI. However, when Talk is embedded in the Files app, the share page or the video verification there is no such element, so the container for the components could not be set and thus they were not shown. To solve this now the selector for the main container element is got from the store instead of being hardcoded, and the different UI modes set the appropriate value when initialized (or leave it undefined to use the default one, typically the body element, when a specific element is not needed). Note that this change applies too to some components that, right now, are only shown in the main Talk UI, but it was done for consistency and to make them "future-proof". Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
force-pushed
the
fix-container-of-modal-and-popup-components-when-talk-is-embedded
branch
from
6dfc8d1
to
2d26b6c
Compare
Yeah totally fine. If someone asks the answer is: "Sorry wasn't possible to fix it there, please update to 11+ for the latest fixes" |
I know other menus also have the problem, but is this somehow fixable, e.g. by using the talk frame as reference (but that would break for notifications then I guess? (Ref nextcloud/server#23872) |
Using the default container causes the action menus in the Talk tab to be repositioned at wrong places when the menus are shown and the file list is scrolled. To address this (although it does not fully fix the issue, there are still some strange behaviours) the main container for Talk components used when Talk is embedded in the Files app is Talk tab. Besides that, both the container and the boundaries element of the actions are set to the Talk tab. Despite setting the main container this change does not affect other components (like the room selector) or slightly improves their behaviour (like the author avatar menu, which no longer appears outside the tab when scrolling). Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Mmm, interesting, scrolling opens a whole new world of quirks :-P Theoretically the menus should use the body or the content as their container, as it would ensure that even if the menu overflows the Talk frame it would not be cropped by its limits. On the other hand, in some cases it would make sense if the menu was limited only to certain region, for example when scrolling. As the menus are not very large and they are shown above or below the trigger based on the available space probably they will not overflow the Talk tab when normally shown, so it might be possible to set the Talk tab as its container to prevent the overflow when scrolling. But... that does not work. Even if Talk tab is not scrolled if the page is scrolled and then a menu is shown the menu might not be visible due to being placed at the top (like in the screenshot) and thus overflowing the Talk tab. It seems that the position of the menu is calculated based on the However, if only So what happens if both In any case, that seems to be the best that can be done for the message menu 🤷 Surprisingly, the menu in the author avatar seems to work fine (except for appearing in front of the top of the sidebar when scrolling) without any of those changes, and just setting the container fixes the overflow without affecting the position. When the Talk tab is used the author avatar menu appears in front of the Join call button when scrolling, but even if the chat view rather than the Talk tab is set as the container the menu still appears in front. Also setting the container of the room selector to the Talk tab is not a problem; the room selector is visible despite appearing outside the Talk tab. I do not know what CSS black magic is used there. Finally, despite setting the container and the boundaries element to the same element, if the message menu is shown in the main Talk UI and the chat view is scrolled the message menu will jump to the top left. This also happens in master (but other menus, like the menus in the conversation list, work fine). |
|
Okay, was just a guess. It's better than before, so let's get it in and hope people don't scroll too much |
Signed-off-by: Joas Schilling <[email protected]>
nickvergessen
deleted the
fix-container-of-modal-and-popup-components-when-talk-is-embedded
branch
|
The backport to stable21 failed. Please do this backport manually. |
|
Ignoring 21 for now |
The container of modal and popup components was always set to "#content-vue" to ensure that they will be properly shown in normal and fullscreen mode in the main Talk UI. However, when Talk is embedded in the Files app, the share page or the video verification there is no such element, so the container for the components could not be set and thus they were not shown.
To solve this now the selector for the main container element is got from the store instead of being hardcoded, and the different UI modes set the appropriate value when initialized (or leave it undefined to use the default one, typically the body element, when a specific element is not needed).
Additionally this pull request also fixes showing the room selector when in full screen mode in main Talk UI.
Pending:
Given the needed work and that Talk 10 will be soon end-of-life probably the best option would be to just leave the menus and dialogs in fullscreen mode broken in Talk 10 :-(
How to test (scenario 1)
Result with this pull request
The popup and the dialog are shown
Result without this pull request
Neither the popup nor the dialog are shown
How to test (scenario 2)
Result with this pull request
The room selector dialog is shown
Result without this pull request
The room selector dialog is not shown