-
Notifications
You must be signed in to change notification settings - Fork 62
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
PLAYER-4682_Two new activities for NPAW-FW and NPAW-IMA #386
base: dev
Are you sure you want to change the base?
PLAYER-4682_Two new activities for NPAW-FW and NPAW-IMA #386
Conversation
Could you please make further pull requests from the branches which belong to this repository (not your own fork)? |
@@ -58,4 +58,6 @@ dependencies { | |||
implementation files('libs/android_accessenabler-1.7.3.jar') | |||
implementation files('libs/YouboraLib-5.3.1.jar') | |||
implementation files('libs/YouboraPluginOoyala-5.3.0.jar') | |||
compile files('libs/YouboraLib-5.3.1.jar') | |||
compile files('libs/YouboraPluginOoyala-5.3.0.jar') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"compile" is obsolete, please use "implementation" or "api"
https://medium.com/mindorks/implementation-vs-api-in-gradle-3-0-494c817a6fa
@@ -177,6 +177,20 @@ | |||
<activity android:name=".players.ProgrammaticVolumePlayerActivity" | |||
android:configChanges="keyboardHidden|orientation|screenSize"> | |||
</activity> | |||
<activity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, activities should be formatted like that:
<activity
android:name="com.ooyala.sample.lists.NPAWFreewheelListActivity"
android:configChanges="keyboardHidden|orientation|screenSize" />
* This class asks permission for WRITE_EXTERNAL_STORAGE. We need it for automation hooks | ||
* as we need to write into the SD card and automation will parse this file. | ||
*/ | ||
public abstract class NPAWFWAbstractHookActivity extends Activity implements Observer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation is broken in this file. We use 2 spaces.
android:paddingLeft="@dimen/activity_horizontal_margin" | ||
android:paddingRight="@dimen/activity_horizontal_margin" | ||
android:paddingTop="@dimen/activity_vertical_margin" | ||
android:orientation="vertical" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
context is not MainActivity
@@ -0,0 +1,27 @@ | |||
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need LinearLayout for a layout with one child.
* - Site Section ID | ||
* | ||
*/ | ||
public class NPAWPreconfiguredFreewheelPlayerActivity extends NPAWFWAbstractHookActivity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting
public class NPAWPreconfiguredFreewheelPlayerActivity extends NPAWFWAbstractHookActivity { | ||
|
||
protected OptimizedOoyalaPlayerLayoutController playerLayoutController; | ||
private PluginOoyala youboraPluginOoyala; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need these fields?
YBLog.setDebugLevel(YBLog.YBLogLevelHTTPRequests); | ||
Map<String, Object> youboraOptions = YouboraConfigManager.getYouboraConfig(getApplicationContext()); | ||
youboraPluginOoyala = new PluginOoyala(youboraOptions); | ||
youboraPluginOoyala.startMonitoring(player); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we start monitoring we need to pause/stop it when the Activity is paused/destroyed.
OoyalaPlayerLayout playerLayout = (OoyalaPlayerLayout) findViewById(R.id.ooyalaPlayer); | ||
playerLayoutController = new OptimizedOoyalaPlayerLayoutController(playerLayout, player); | ||
|
||
@SuppressWarnings("unused") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need unused code?
} | ||
|
||
@Override | ||
public void update(Observable o, Object arg) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arg1 -> notificationName
@DmitriiMaslov @SergeyBicarte
Please review the two new below activities created for CompleteSampleApp and merge into dev branch.