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

chore: mirror player.src on the demo page demo page #1071

Merged
merged 2 commits into from
Feb 17, 2021

Conversation

brandonocasey
Copy link
Contributor

Description

Uses sourceset to mirror the source that was set via player.src in the ui so that it will be saved on page reload!

gkatsev
gkatsev previously approved these changes Feb 16, 2021
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming aside, LGTM

index.html Outdated
@@ -88,6 +88,10 @@ <h3>Options</h3>
<input id=override-native type="checkbox" checked>
Override Native (reloads player)
</label>
<label>
<input id=mirror-source type="checkbox" checked>
Mirror sources from player.src (reloads player)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just call this sourceset? This the target of these pages are us, we can be as technical as we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I wanted to, but that isn't really the feature that we care about here. We care about mirroring sources from player.src, sourceset just happens to make that very easy to do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe adding uses sourceset in the parens too.
My thought process here is that we always want mirroring but that it's possible that sourceset may cause issues so we'd want to disable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do that and merge

@brandonocasey brandonocasey merged commit a53e1d0 into main Feb 17, 2021
@gkatsev gkatsev deleted the chore/demo-player-src branch May 17, 2021 21:43
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.

2 participants