Add an ActionBar entry for adding a command to the list. The current way to add commands is quite hidden.
Details
Details
- Reviewers
mtijink - Group Reviewers
KDE Connect - Commits
- R225:ae0538ae0c03: Make it possible to add commands for the RunCommandsPlugin from Android
Diff Detail
Diff Detail
- Repository
- R225 KDE Connect - Android application
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
Adapt to change in desktop part. Also show the plugin even if the commandlist is empty to make users aware that this functionality exists.
Comment Actions
Works good, but I still have some comments.
| res/layout/addcommanddialog.xml | ||
|---|---|---|
| 18 | I'd add some margin to these fields. The android docs use 4dp. | |
| res/menu/menu_runcommand.xml | ||
| 1 ↗ | (On Diff #20829) | Can we use a Floating-Action-Button instead of a menu button? Or did you specifically choose the menu button? |
| 5 ↗ | (On Diff #20829) | Can you use the vector asset instead? |
| src/org/kde/kdeconnect/Plugins/RunCommandPlugin/AddCommandDialog.java | ||
| 40 | Would it be possible to disable the ok button while either input field is still empty? | |
| 54 | Why is this empty if here? | |
| src/org/kde/kdeconnect/Plugins/RunCommandPlugin/RunCommandActivity.java | ||
| 47 | theCallback is a weird name, I'd suggest addCommandDialogCallback or similar. | |
| src/org/kde/kdeconnect/Plugins/RunCommandPlugin/RunCommandPlugin.java | ||
| 141 | So if the desktop does not support remotely adding commands, the (empty) activity still appears, without anything to do. Can we hide it in that case, or add a text displaying how to add commands on the desktop? | |
Comment Actions
Implement suggestions
| src/org/kde/kdeconnect/Plugins/RunCommandPlugin/AddCommandDialog.java | ||
|---|---|---|
| 40 | In theory, yes, but I couldn't make it work perfectly. Also the button has no visual feedback whether it's disabled or not | |
| src/org/kde/kdeconnect/Plugins/RunCommandPlugin/RunCommandPlugin.java | ||
| 141 | My reason to unhide it was to show users that this functionality exists, The text is a great idea | |
Comment Actions
Looks good! Can be merged, as far as I'm concerned, after the fix for the crash.
| res/layout/activity_runcommand.xml | ||
|---|---|---|
| 27 | Needs to be app:backgroundTint or it crashes for me. | |
| res/layout/addcommanddialog.xml | ||
|---|---|---|
| 15 | This needs API 21. Does anyone know if this causes serious problems on old devices or will it just be ignored? | |
| res/layout/addcommanddialog.xml | ||
|---|---|---|
| 15 | It'll cause an inflation exception at runtime. There should be a backwards-compatible alternative provided with the android support libraries, available as @style/TextAppearance.AppCompat.Medium. | |