fix(watermarks): selectively enable watermarks

[joshuai96]

The z-index of the PremeetingScreen is 252 while the watermarks only have the z-index of 2 or 100 for the "powered by". This hides the watermarks behind the content panel.

If the watermarks are hidden by design here, they should be conditionally excluded from the DOM, as they can be navigated to via keyboard. This breaks the sequential navigation required for accessibility.

I opted to bring the Watermarks to the top, as determining which view is currently active (prejoin, lobby or active conference) and bringing this information down to Watermarks (Conference -> LargeVideo -> Watermarks) seems a lot more hassle, but can be implemented that way, too.

[jitsi-jenkins]

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

[damencho]

That does not look aligned with the prejoin screen.

[damencho]

I guess its intentionally hidden.

[joshuai96]

Then this may should be hidden for real or excluded from the DOM. As I mentioned, this breaks the sequential navigation via keyboard, as it gets focused, but is hidden behind the content panel.

I will change the PR as soon as I get a fix for it.

[joshuai96]

This patch works for our case and satisfies the accessibility requirements.

The condition for loading the watermarks could be extended, but only the prejoin screen was tested for us.

[damencho]

@joshuai96 have you signed the CLA, as I don't see it at the moment?

[joshuai96]

@damencho we are currently in the process of evaluating and signing the CCLA and ICLA.

[damencho]

In the meantime, if you disable pre-join and enable lobby for the room you will experience the same problem.

jitsi-meet/react/features/conference/components/web/Conference.tsx

Line 268 in b352006

{ (_showPrejoin && !_showVisitorsQueue) && <Prejoin />}

So I guess the same can be applied when getIsLobbyVisible is true.

[joshuai96]

I've extended the condition to exclude the watermarks on lobby and prejoin.

[damencho]

jenkins test this please

[saghul]

Not sure if this was a mistake or intentional. We should check with design.

[joshuai96]

@damencho ICLA and CCLA are signed now.

[saghul]

FWIW, @emcho had no objections to showing the logo on the prejoin screen.

[joshuai96]

FWIW, @emcho had no objections to showing the logo on the prejoin screen.

Having the logo on the prejoin screen looks like a side product of loading a lot of conference components before even joining the call.

We are currently doing a lot of accessibility work, so in that regard, it may be more beneficial to keep the component excluded in the prejoin screen in favor of navigating the screen via keyboard more quickly and easily. A lot of "visitable" components means a lot more mental strain, as the in mind "map" of navigation grows and determining the important components (join call) gets more difficult.

[saghul]

From a branding prespective, the prejoin screen has zero logos, which can be seen as kind of wrong. If it's the first time you are using the product, and the first thing you see is the pre-join screen because you are just following a URL, there not a single logo visible.

[joshuai96]

Valid point from branding perspective. I'm gonna look into changing the style to get them on top again.

I've noticed the inconsistency between scss styles and in component style objects. Is there a preferred way?

The scss define some z-index values for different usages. But you can't use them in the style object. Is there maybe any better way then hacking the same values into the object? Is there any afford done to produce some reusable style definitions, either in scss or style object (which ever is preferred)?

Sorry, this is more of a general question and might be part of a more general discussion on this topic.

[saghul]

I've noticed the inconsistency between scss styles and in component style objects. Is there a preferred way?

Component style objects are what we are transitioning to.

The scss define some z-index values for different usages. But you can't use them in the style object. Is there maybe any better way then hacking the same values into the object? Is there any afford done to produce some reusable style definitions, either in scss or style object (which ever is preferred)?

Sorry, this is more of a general question and might be part of a more general discussion on this topic.

Can't we make this work by upping the z-index in the scss file?

[joshuai96]

Component style objects are what we are transitioning to.

Noted for further changes.

Can't we make this work by upping the z-index in the scss file?

Yes, that was exactly what my first approach was. I changed it later to the component exclusion. Like I said: It's more of a general question, for further changes and probably belongs in a separate discussion.

[saghul]

Yes, that was exactly what my first approach was. I changed it later to the component exclusion. Like I said: It's more of a general question, for further changes and probably belongs in a separate discussion.

I see. Then let'd do z-rder to start with, and go on from there.