-
Notifications
You must be signed in to change notification settings - Fork 61
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)? |
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
<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.