Updated atlaskit components

[skolmer]

This PR updates all atlaskit dependencies.
This will increase accessibility of jitsi-meet because the new atlaskit components contain required aria attributes and other accessibility improvements. They also improved performance and responsiveness of some components.

If you are confused about the webpack config changes. The atlaskit team missed to add dark theme support to modal dialogs so we are injecting the required props to this component.

If this PR gets accepted we would like to provide an additional PR to complete support of WCAG 2.1 guidelines by fixing accessibility issues that are not related to atlaskit.

To fully match WCAG 2.1 we also need to re-enable focus-lock that was disabled in this commit (3c1f056). This might be possible using a newer feature of the library: theKashey/react-focus-lock#104
Any thoughts about this?

[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 :(.

[skolmer]

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 :(.

CLA was already signed for this PR -> #7902

[Simon818]

Not sure if this is appropriate here, but just wanted to say thanks for doing this. I think there are a lot of people holding off on switching because of a lack of ARIA and other accessibility-related concerns, and that seems like a shame.

[saghul]

Impressive work! We have some AtlasKit overrides so making sure everything is still working as intended may take a bit, but I'm very excited to see this!

[skolmer]

Impressive work! We have some AtlasKit overrides so making sure everything is still working as intended may take a bit, but I'm very excited to see this!

Thanks! Definitely a good idea to do some additional testing. I made sure though that everything is still working as expected (compared against meet.jit.si) but always easy to miss a detail when making such a change.
Just let us know when there is anything we can help with.
Our client is currently doing another round of WCAG 2.1 compatibility checks. If anything else related to atlaskit comes up there I will update the PR.

[saghul]

I made a quick pass and it's almost there! Things I noticed:

"GSM bars" popover is not correctly positioned (then vs now)

Remote participant menu is not correctly positioned (then vs now).

I think both may have the same root cause. I think in general all overrides in _atlaskit_overrides.scss may need a revisit.

[saghul]

Tip: due to how large the changes in package-lock are you may want to skip that file until we are ready to merge.

[skolmer]

Ok, looks like I missed some styles in _atlaskit_overrides.scss. I will fix that today and let you know when it's ready.

[saghul]

If this PR gets accepted we would like to provide an additional PR to complete support of WCAG 2.1 guidelines by fixing accessibility issues that are not related to atlaskit.

To fully match WCAG 2.1 we also need to re-enable focus-lock that was disabled in this commit (3c1f056). This might be possible using a newer feature of the library: theKashey/react-focus-lock#104
Any thoughts about this?

Sorry I forgot to reply to this. Let's discuss extra individual items in dedicated PRs once AtlasKit is updated, how does that sound?

[skolmer]

@saghul There was a breaking change in InlineDialog that I missed. This is now fixed too.

I'm still working on _atlaskit_overrides.scss to check that every style in there gets applied. This will follow in another commit.

[skolmer]

_atlaskit_overrides.scss has now been cleaned up too. Some styles were obsolete because it was already fixed in atlaskit and others had been reworked to not rely on those generic class names that might change in the future. The PR is now ready for a final review @saghul

[skolmer]

Ok, I somehow expected that the new npm version will cause trouble at some point. Looks like I have to downgrade to update the lockfile.

@saghul @muscat1 Did you find the time to retest, anything else that came up?

[saghul]

I haven't had the time to take a second look, sorry. What Node / npm version did you use? We use Node 12 and npm 6. @muscat1 can you please check again tomorrow?

[skolmer]

I haven't had the time to take a second look, sorry. What Node / npm version did you use? We use Node 12 and npm 6. @muscat1 can you please check again tomorrow?

I installed node 15 a few days ago. That wasn't a good idea because they changed the lockfile version for the first time in years.

[muscat1]

Tested the use cases I've mentioned above and they have been addressed, thanks for looking into them! From my perspective, things seem to be in order, unless you can find some other irregularities, @saghul

There do seem to be some linting problems, though :D

[skolmer]

Tested the use cases I've mentioned above and they have been addressed, thanks for looking into them! From my perspective, things seem to be in order, unless you can find some other irregularities, @saghul

There do seem to be some linting problems, though :D

Thanks @muscat1, I will look into the linting issues.

[saghul]

Jenkins please test this please.

[saghul]

Squashed, rebased and added package-lock in #8480

[saghul]

Jenkins please test this please.

[saghul]

@skolmer The test failures seem "legit", you can check the selenium output on the details link. It says the element is not interactable, but I couldn't figure out why on a quick test. Can you please take a look?

[skolmer]

@saghul Can you point me to the code of the failing selenium test? Maybe it is just because of changed html structure.

[saghul]

The red ones here: http://pr-reports.jitsi.org/jitsi-meet-PR-test/8557/chrome-chrome-reports/html/ they both originate in this function: https://github.com/jitsi/jitsi-meet-torture/blob/04a0c13f950228bf907673a08c9ed5169cc3211a/src/test/java/org/jitsi/meet/test/pageobjects/web/SettingsDialog.java#L215

[skolmer]

@saghul This is a tricky one. In the old atlaskit checkbox the input field had opacity: 0 and was positioned behind the svg. In the new one it has appearance: none which makes it inaccessible to selenium. I think there are a few possible solutions:

1. Change the checkbox selenium test to override the apperance style before using webelement.click()
2. Change the checkbox selenium test to use javascript click instead of webelement.click()
3. Override css styles of input[type="checkbox"] to us the old approach of previous atlaskit versions:

    left: 50%;
    margin: 0px;
    opacity: 0;
    padding: 0px;
    position: absolute;
    transform: translate(-50%, -50%);
    top: 50%;

This is the line that is failing: https://github.com/jitsi/jitsi-meet-torture/blob/master/src/test/java/org/jitsi/meet/test/pageobjects/web/SettingsDialog.java#L215

[saghul]

I'm thinking 2 sounds like a good approach here, since it would work with both current and after-your-PR changes, while staying away from more AtlasKit overrides. Thoughts @muscat1 ?

[skolmer]

2 would be ((JavascriptExecutor)driver).executeScript("arguments[0].click();", checkbox); instead of checkbox.click(); https://github.com/jitsi/jitsi-meet-torture/blob/69e5b6e123b98842b58bdf156ba5044f3695cb6d/src/test/java/org/jitsi/meet/test/pageobjects/web/SettingsDialog.java#L215

[saghul]

Tested locally and works fine, mind making a PR to the torture project?

[damencho]

If you update torture, will that work for the old versions?

[skolmer]

If you update torture, will that work for the old versions?

I haven't run the tests against the old version of atlaskit but judging based on the HTML structure it would work.

[saghul]

If you update torture, will that work for the old versions?

Yeah

[damencho]

Ok, thanks.

[saghul]

Landed in #8480. Amazing work @skolmer , thank you so much!