element-hq / element-web

A glossy Matrix collaboration client for the web.
https://element.io
GNU Affero General Public License v3.0
11.14k stars 1.98k forks source link

Create a screenshot test for voice message player #24747

Open luixxiul opened 1 year ago

luixxiul commented 1 year ago

Steps to reproduce

  1. Create a PR to fix the current implementation of Player.

Outcome

What did you expect?

There should be a screenshot test for it.

What happened instead?

There is not a test, and a manual review is required.

The lack of a screenshot test has really discouraged us to fix visual bugs of audio player and voice message player.

See: https://github.com/vector-im/element-web/issues/22541#issuecomment-1431000971, https://github.com/matrix-org/matrix-react-sdk/pull/8861#issuecomment-1167351433, https://github.com/matrix-org/matrix-react-sdk/pull/8815#issuecomment-1156823559, and https://github.com/matrix-org/matrix-react-sdk/pull/8861#issuecomment-1437058404

This area is highly sensitive design wise, so per other PRs I'm intending to take a look before it lands.

It's not ideal that we currently don't have screenshot tests for voice messages.

We have very strict tolerances on the padding/margins for the voice message components though - I'm going to look into adding screenshot tests for this area.

Operating system

No response

Browser information

No response

URL for webapp

No response

Application version

No response

Homeserver

No response

Will you send logs?

No

luixxiul commented 1 year ago

https://github.com/matrix-org/matrix-react-sdk/pull/10400 lands an initial test of the audio player on FilePanel, although it is not comprehensive.

luixxiul commented 1 year ago

audio-player.spec.ts has been implemented.