feat(Participant) - count total talking time within call for participants

[Antreesy]

☑️ Resolves

• Close Show "Talking time" in participants list during a call #9903
• Known issues:
  • Signaling server doesn't notify for leaving participants - they're still could 'speaking' for 5 seconds
• Follow-ups:
  • Share speaking time information between models on connection
  • Test on force reconections feat(Participant) - count total talking time within call for participants #10068 (review)

🖼️ Screenshots

🏚️ Before	🏡 After
For currently speaking:

🚧 Tasks

• Code review
• Design review
  • Additional highlight for currently speaking partcipants (apart from different text and bold font-weight)?
• Test fix

🏁 Checklist

• ⛑️ Tests (unit and/or integration) are included or not required
• 📘 API documentation in docs/ has been updated or is not required
• 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required
• 🔖 Capability is added or not needed

[danxuliu]

LocalVideo and Video views should not act as proxies of the events of their models. The source of truth is the model, and a view may or may not exist independently of its model; different view objects can be created and destroyed during the lifetime of the model, and there could even be several views for the same model (maybe right now there is always a single view for each participant model, I have not checked, but that would be just a happy coincidence).

Following a similar reasoning, the speaking time should not be calculated directly on the Participant component; it should be calculated in a helper object (one for all participants? One for each participant? I have no preference on that) that directly listens to the speaking events from the model and any view that wants to know the current speaking time of certain participant just gets the value from that helper object (whether it is exposed through an attribute that is read using reactivity or by emitting an event is not really relevant, the important part is that the speaking time is calculated/tracked in that helper object rather than the view).

[ShGKme]

Looks super in general!

Some minor code style comments.

About design:

1. Seconds don't update at the same time that looks not perfect :D
   But I don't know how easily make them sync without putting it to the store and commiting mutation every second.
2. Time has the same text and time while talking and while not.
   "💬 01:20 talking time" - is it "talking now" and "was talking"? You will know only in a second
   What about making different styles for "speaking" and "not speaking" state?

[danxuliu]

Besides the comments personally I would not even use the store and expose the speaking time directly in attributes in the SignalingSpeakingHandler (like done for other models). But in this case it would require an associative array for the participants hashed by (Nextcloud) session ID or something like that, which I do not know if it could be problematic for reactivity. Anyway, that is just personal preference... because I dislike stores :-P

[jancborchardt]

Nice! To comment on @ShGKme's points:

Seconds don't update at the same time that looks not perfect :D

I think this is fine, usually only 1 person talks at a time – or there is an issue. ;)

Time has the same text and time while talking and while not.
What about making different styles for "speaking" and "not speaking" state?

Makes sense – how about giving the avatar of the current speaker a white border like we do for the video containers in the view, making it consistent? :) Additionally the text can be bolded as well, like we do in the video containers too.

[Antreesy]

@jancborchardt please see updated screenshots at the PR description:

• bold font-weight for text of currently speaking participant;
• color-primary-element outline for the avatar (white doesn't seem to have a proper visibility on background)

@danxuliu the main logic of keeping elapsed time data and its calculation remains in store (until the follow-up is resolved), handling of signals was updated, please take a look as well

[danxuliu]

Tested and mostly works 👍

I found a little issue: after a forced reconnection the talking time for the local participant is no longer shown in the sidebar (although the store is still updated when speaking and stopping speaking, as can be seen with store.getters.getParticipantSpeakingInformation(TOKEN, store.getters.getSessionId())). This can be tested in the following way:

• Add a function to force reconnect from the console in src/utils/webrtc/webrtc.js:

	webrtc.forceReconnect = function() {
		forceReconnect(signaling)
	}

• Rebuild the code
• Setup the HPB
• Start a call
• Start speaking
• In the browser console, run OCA.Talk.SimpleWebRTC.webrtc.forceReconnect()

Anyway all this could be fixed in a follow up if needed, as it may not be directly related to the speaking indicator but how sessions are handled after forced reconnections or something like that.

Finally, as a little nitpick I would have added the highlighted border for speaking participants in a separate commit :-P

[Antreesy]

Anyway all this could be fixed in a follow up if needed, as it may not be directly related to the speaking indicator but how sessions are handled after forced reconnections or something like that.

Let's take a look after the feature freeze, as the current state works fine to provide screenshots for marketing, and changes shouldn't touch anything UI-related

Finally, as a little nitpick I would have added the highlighted border for speaking participants in a separate commit :-P

I've asked for the final design review, to see, if that's needed. Personally. font-weight: bold and blue border around avatar look good enough for me

[danxuliu]

Other than the mentioned issue, tested and works 👍

Anyway all this could be fixed in a follow up if needed, as it may not be directly related to the speaking indicator but how sessions are handled after forced reconnections or something like that.

Let's take a look after the feature freeze, as the current state works fine to provide screenshots for marketing, and changes shouldn't touch anything UI-related

Totally fine to wait.

Finally, as a little nitpick I would have added the highlighted border for speaking participants in a separate commit :-P

I've asked for the final design review, to see, if that's needed. Personally. font-weight: bold and blue border around avatar look good enough for me

Sorry, I did not explain myself clearly 🤦 With the highlighted border I was referring to the bold font and the blue border around the avatar (which I also think that look good enough); it was just a comment about preferring that to be split from the code changes to add the speaking indicator, not about adding any additional design change :-)

[danxuliu]

Now that I see that... it should not be called SignalingSpeakingHandler :-) It should be called something like SpeakingStatusHandler or SpeakingStatusNotifier or SpeakingStatusAdapter or SpeakingStatusStoreAdapter or... whatever (I can not decide, and I could think of more names xD), but not Signaling :-)

And it should be in src/utils/webrtc rather than src/utils, as it is specific to calls rather than a generic utility.

[DorraJaouad]

Tested and it works 🦅.

[jancborchardt]

Looks good design-wise, nice! :)

[Antreesy]

Splitted the last commit into two, renamed and moved handler, fix comments and computed properties wording - no significant changes

[nickvergessen]

/backport to stable27

[danxuliu]

Tested again after the last rebase and (still :-) ) works 👍

CI failure is unrelated (chat integration tests in PostgreSQL)