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

Refactor to use track-started and track-stopped #9

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

kimberleehowley
Copy link
Contributor

@kimberleehowley kimberleehowley commented Feb 23, 2021

ETA: This is now ready for review (see comments below).

@kimberleehowley
Copy link
Contributor Author

Hi, @jessmitch42!

@philmillman mentioned that you might have some time tomorrow to revisit our original React demo refactoring. I hope this tag is okay!

There are two main things I am struggling with: 

  1. The shared screen for the person screensharing is backwards
  2. When I stop a screenshare, the black tile remains. Because, I think, when a participant stops sharing their screen, 'participant-updated' fires along with the 'track-stopped'. Then addOrUpdate() participant runs with empty track state being passed to the tile. 

I've tried, among other things: 

  • Checking the state of the track within addOrUpdate(), but I must be doing something wrong. 
  • Exiting out of handleParticipantUpdate under various conditions, to not much luck (aside from my workaround to make sure the shared screen displays). 
  • Changing the id of the tile to the track id, but that was a bust. Instead, I now look at videoTrackState now to remove the tile.

If you have any ideas for managing the event bubbling (I was looking up stopPropagation for custom events), I'd appreciate it.

And/or, if there's another direction to solve, or if there's a better way to approach from scratch (I'm currently storing a lot of screensharing state), I'm all ears.

Thank you so much!
Kimberlee 

@kimberleehowley
Copy link
Contributor Author

Hi, @jessmitch42 and @kompfner!

At long last, we've got a refactored version of the OG React demo that 1) Relies on track-started and track-stopped to add tracks and 2) No longer needs callState.js or the getCallItems pattern. Jess, thank you again for all of your help!

For this intro demo that doesn't have toooo complicated of a state, I think this setup could be easier to follow for devs newer to Daily and newer devs in general (at least, it was for me). It closely mirrors our party-line pattern.

I successfully tested:

  • 2 participants, one leaving in the middle of the call
  • Screenshare starting and stopping multiple times for 2x participants
  • 3 participants
  • Starting a screenshare with 3 participants
  • Adding a fourth participant, then a fifth
  • Making sure participants could come and go with an ongoing screenshare
  • Checking for correct behavior when participant screensharing leaves

When you have 20 minutes some time next week, I'd appreciate your feedback on:

  1. Is there anything else I should've tested for?
  2. What do you think of this refactoring in general?

Thank you!
Kimberlee

@kimberleehowley kimberleehowley marked this pull request as ready for review March 12, 2021 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant