Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix empty call view position in sidebar #4429

Merged
merged 1 commit into from
Oct 22, 2020

Conversation

danxuliu
Copy link
Member

This is meant to be tested with server in fd178b92f63^ (as Talk master is not yet compatible with the Files sidebar changes introduced in nextcloud/server@fd178b9).

Although this became specially noticeable after #4408 it was already broken in previous versions (as the call view height was reduced when someone else joined the call), hence the backport to stable20 (it also happens in stable19 and stable18, so feel free to backport to them too if you want, but as it is "just" a style issue I only backported to stable20 to begin with).

Before:
Files-Sidebar-Empty-Call-Before

After:
Files-Sidebar-Empty-Call-After

When the call view is shown in the Files app or public share page
sidebar its direct children (the empty call view and the wrapper of
videos) are expected to have an absolute position. However, as the empty
call view had a relative position it was prepended before the call view
padding (which defines the call view height) instead of fully covering
it.

When the call view is shown in the main Talk UI or in the video
verification sidebar the call view has an explicit height attribute
(rather than using a padding), so in this case it does not matter if the
empty call view has a relative or absolute position.

However, when the empty call view is shown in the grid it must use a
relative position.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu added 3. to review bug feature: frontend 🖌️ "Web UI" client feature: talk-sidebar ⬅️ Sidebar integration of Talk into other apps like sharing and documents feature: call 📹 Voice and video calls labels Oct 21, 2020
@danxuliu danxuliu added this to the 💚 Next Major (21) milestone Oct 21, 2020
@danxuliu
Copy link
Member Author

/backport to stable20

@PVince81
Copy link
Member

not reproducible on master for me, neither in Firefox nor Chromium:
image

I tried both making a call as guest and as regular user.
And I think @ma12-co would have noticed as well when reviewing the collapsible stripe (which needed a lot of clicking around)

what browser are you using ? I never noticed any call height changes before.

@danxuliu
Copy link
Member Author

what browser are you using ? I never noticed any call height changes before.

Both Firefox and Chromium. But your screenshot is from the main Talk UI, not from a call in the Files app or the public share page sidebar which are the ones being fixed here ;-)

@PVince81
Copy link
Member

oh... when I tested the sidebar mode I thought there was only a single video in the sidebar.
how do you switch to that "fullscreen mode from sidebar" ?

@danxuliu
Copy link
Member Author

oh... when I tested the sidebar mode I thought there was only a single video in the sidebar.
how do you switch to that "fullscreen mode from sidebar" ?

There is no fullscreen mode from sidebar, I just cropped the screenshot to the call view area :-) See the tabs and the Leave call button below it ;-)

@PVince81
Copy link
Member

I tried with NC 20 and can indeed see that the waiting screen is higher. Cool, now on the same page.

@PVince81
Copy link
Member

PVince81 commented Oct 22, 2020

I'll check these:

  • test with stable20
  • test with talk master (merged with sidebar PR)
    • verify fix
    • regression check in call view
      • waiting for
      • one to one
      • one to many stripe mode
      • collapsed mode
      • grid mode

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, works 👍

@PVince81 PVince81 merged commit 825ffb6 into master Oct 22, 2020
@PVince81 PVince81 deleted the fix-empty-call-view-position-in-sidebar branch October 22, 2020 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug feature: call 📹 Voice and video calls feature: frontend 🖌️ "Web UI" client feature: talk-sidebar ⬅️ Sidebar integration of Talk into other apps like sharing and documents
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants