D12507: Added runcommandplugin widget

[Nicolas Fella]

nicolasfella requested changes to this revision.
nicolasfella added a comment.
This revision now requires changes to proceed.


  Working pretty good so far.
  The command list does not seem to get updated when switching the selected device.
  Some design nitpicks as inline comments.
  Please respect the coding style of the rest of the project, i.e. no spaces inside braces. We are using the standard settings of Android Studio, so you can use the builtin formatter.

INLINE COMMENTS

> widget_remotecommandplugin.xml:4
> +    android:orientation="vertical" android:layout_width="match_parent"
> +    android:background="#bbFFFFFF"
> +    android:layout_height="match_parent">

The transparent background is weird IMHO, it makes the command text hardly readable

> widget_remotecommandplugin.xml:13
> +
> +        <ImageView
> +            android:layout_width="wrap_content"

I would like a little bit of padding below and above

> widget_remotecommandplugin.xml:35
> +        android:orientation="vertical" />
> +    <TextView
> +        android:id="@+id/not_reachable_message"

Some padding to the left to make it symmetrical

> RunCommandPlugin.java:48
>      private ArrayList<CommandsChangedCallback> callbacks = new ArrayList<>();
> +    private final ArrayList< CommandEntry > commandItems = new ArrayList<>();
>  

Coding style, no spaces inside <>.
Apply everywhere

> RunCommandPlugin.java:122
> +
> +                Collections.sort( commandItems, new Comparator<CommandEntry>(){
> +                    @Override

Lambda

> RunCommandWidget.java:41
> +
> +            BackgroundService.RunCommand( context, new BackgroundService.InstanceCallback( ){
> +                @Override

Lambda

> RunCommandWidget.java:84
> +
> +            BackgroundService.RunCommand( context, new BackgroundService.InstanceCallback( ){
> +                @Override

Lambda

> RunCommandWidget.java:121
> +            else{
> +                views.setTextViewText( R.id.runcommandWidgetTitle, DeviceHelper.getDeviceName( context ) + "@" + currentDevice.getName( ) );
> +                views.setViewVisibility( R.id.runcommandslist, View.VISIBLE );

I don't think it's necessary to include the phones name here. In my case it results in "OnePlus at nico@neon", two time @ is too much

> RunCommandWidget.java:150
> +
> +    public static void setCurrentDevice( final Context context, final String DeviceId ){
> +        BackgroundService.RunCommand( context, new BackgroundService.InstanceCallback( ){

Lambda

> RunCommandWidgetDeviceSelector.java:30
> +
> +        BackgroundService.RunCommand(this, new BackgroundService.InstanceCallback() {
> +            @Override

Lambda

> RunCommandWidgetDeviceSelector.java:51
> +
> +                        Collections.sort(commandItems, new Comparator<ListAdapter.Item>() {
> +                            @Override

Lambda?

> RunCommandWidgetDeviceSelector.java:63
> +                        view.setAdapter(adapter);
> +                        view.setOnItemClickListener(new AdapterView.OnItemClickListener() {
> +                            @Override

You can use lambda here

REVISION DETAIL
  https://phabricator.kde.org/D12507

To: menasshock, albertvaka, #kde_connect, nicolasfella
Cc: kdeconnect, nicolasfella, ahmedbesbes, daniel.z.tg, jeanv, MayeulC, menasshock, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20180510/b36ec544/attachment-0001.html>