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

feat: player doesn't attach itself to parent #27

Merged
merged 2 commits into from
Sep 17, 2017
Merged

Conversation

OrenMe
Copy link
Contributor

@OrenMe OrenMe commented Sep 14, 2017

Description of the Changes

only create instance of playkit and pass it to UI as we do today.
UI manager will attach it to DOM as part of entire UI build out.
This depends on kaltura/playkit-js#113 and future PR on playkit-js-ui SDK

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

only create instance of playkit and pass it to UI as we do today.
UI manager will attach it to DOM as part of entire UI build out.
This depends on kaltura/playkit-js#113 and future PR on playkit-js-ui SDK
@dan-ziv
Copy link
Contributor

dan-ziv commented Sep 14, 2017

LGTM
Just need to fix the tests

@OrenMe
Copy link
Contributor Author

OrenMe commented Sep 16, 2017

@dvh91 the tests are failing due to dependncy in UI on player, which should now be responsible on adding the player, so waiting to your change.
@dan-ziv, @yairans, @odedhutzler - I think we should separate the tests between functional/integration/e2e and unit tests.
The failing tests here are integration tests that fail due to 3rd party dependency - which is good to have, but we should separate them somehow.
This is not something to resolve in this PR but we should discuss this matter in one of the dailies.

By the way - what fails in the UI is an explicit call to player DOM element that causes dependency between player and UI, so this change will, as a side effect, remove this dependency.

@OrenMe OrenMe merged commit b612d57 into master Sep 17, 2017
@OrenMe OrenMe deleted the containerRefactor branch September 17, 2017 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants