ui: Implement support for Android Dynamic Shortcuts #55

tturner · 2023-06-11T14:40:38Z

This is an updated version of the pull request #48 to try to make it more likely to be merged.

The hard work was done by @gergely-sallai Gergely Sallai in their fork. I have tweaked it as follows:

update to compile against the current master branch
made a small cosmetic change to the alignment of the UI component
squashed into a single commit for simplicity

I hope this is the right way to get this merged and that I haven't broken any github etiquette!

Original pull request information reproduced below:

Added support for Android Dynamic Shortcuts. Dynamic Shortcuts is a standard Android feature that is supported by most of the launchers.

How does this work for users?
Now users can request the app to add shortcuts to their favourite tunnels. When the user enables this for a specific tunnel, 2 dynamic shortcuts are created, one to UP the tunnel, other is to DOWN it.

Additionally this functionality enables users to use certain automation apps to automatically toggle tunnels. For example the built-in Bixby Routines on Samsung devices. (This was my original motivation for adding this feature, but I was trying to incorporate it in a way that could benefit all users.)

What changes were made to previous components to support this?
TunnelToggleActivity is now more broadly used. Previously it seemed to be only called from TileService in certain situations.
A new button has been added next to the tunnel toggle switch for each tunnel item.
Added androidx shortcuts library
Limitations and possible points for future improvement
To my best knowledge all of these -- expect the first point -- were there previous to my PR, and I can help to sort these out if needed.

Tooltip popup for the "add shortcuts" button is only available for devices running API 26 and up. This could be solved generally via TooltipCompat, but I didn't want to complicate the code because of this. As doing this with the compat lib can not be done purely from XML..
TunnelToggleActivity can be shown briefly when a tunnel is toggled, and it can look weird. This could possibly be solved by adding some design to the activity and showing it a bit longer in a controlled manner, maybe with some animation.
There may be some misconfigurations around material colors, it caused me some rendering differences on certain OS version with the button colors. I believe this could be solved with a refactor to support Material You (M3) and its dynamic colors, and fall back to a default on older versions. I do not know what is the intended design language of the app, so I didn't dare to refactor it just yet. :)

This work was done by Gergely Sallai in their fork. I have tweaked it as follows: - update to compile against the current master branch - made a small cosmetic change to the alignment of the UI component - squashed into a single commit for simplicity Signed-off-by: Tom Turner <[email protected]>

zx2c4 · 2025-05-12T14:47:10Z

ui/src/main/java/com/wireguard/android/model/ObservableTunnel.kt

+    var hasShortcut: Boolean? = null
+        get() {
+            if (field == null)
+            // Opportunistically fetch this if we don't have a cached one, and rely on data bindings to update it eventually


Comment indentation looks a little funny.

zx2c4 · 2025-05-12T14:47:38Z

ui/src/main/java/com/wireguard/android/model/ShortcutManager.kt

+    private fun downIdFor(name: String): String = "$name-DOWN"
+
+    private fun createShortcutIntent(action: String, tunnelName: String): Intent =
+        Intent(context, TunnelToggleActivity::class.java).apply {


Also weird indentation here?

zx2c4 · 2025-05-12T14:48:03Z

ui/src/main/java/com/wireguard/android/activity/TunnelToggleActivity.kt

        lifecycleScope.launch {
+            val tunnelAction = when(intent.action) {
+                "com.wireguard.android.action.SET_TUNNEL_UP" -> Tunnel.State.UP


Should these strings be constants somewhere?

zx2c4 · 2025-05-12T14:48:20Z

ui/src/main/java/com/wireguard/android/activity/TunnelToggleActivity.kt

+            val tunnelAction = when(intent.action) {
+                "com.wireguard.android.action.SET_TUNNEL_UP" -> Tunnel.State.UP
+                "com.wireguard.android.action.SET_TUNNEL_DOWN" -> Tunnel.State.DOWN
+                else -> Tunnel.State.TOGGLE // Implicit toggle to keep previous behaviour


"Keep previous behavior" is not a useful code comment. Just remove that comment.

zx2c4 · 2025-05-12T14:48:42Z

ui/src/main/java/com/wireguard/android/activity/TunnelToggleActivity.kt

+    /**
+     * TileService is only available for API 24+, if it's available it'll be updated,
+     * otherwise it's ignored.
+     */


This comment doesn't help much either. Get rid of it.

zx2c4 · 2025-05-12T14:49:47Z

ui/src/main/java/com/wireguard/android/fragment/BaseFragment.kt

@@ -108,6 +109,18 @@ abstract class BaseFragment : Fragment(), OnSelectedTunnelChangedListener {
        }
    }

+    fun setShortcutState(view: CompoundButton, checked: Boolean) {


Can that just be a generic View? We don't actually need any CompoundButton members, right?

zx2c4 · 2025-05-12T14:51:18Z

ui/src/main/java/com/wireguard/android/model/ShortcutManager.kt

+    }
+
+    fun hasShortcut(name: String) =
+        ShortcutManagerCompat.getDynamicShortcuts(context).any { it.id.startsWith(name) }


startsWith is not a safe thing here, because tunnel names can be prefixes of each other.

zx2c4 · 2025-05-12T14:51:55Z

ui/src/main/java/com/wireguard/android/model/TunnelManager.kt

@@ -65,6 +66,10 @@ class TunnelManager(private val configStore: ConfigStore) : BaseObservable() {
        // Make sure nothing touches the tunnel.
        if (wasLastUsed)
            lastUsedTunnel = null
+        // Make sure we also remove any existing shortcuts.
+        if (shortcutManager.hasShortcut(tunnel.name)) {
+            shortcutManager.removeShortcuts(tunnel.name)


Can't we just call remove unconditionally, and it'll be a no-op if they're not already there? What do we gain by first checking?

zx2c4 · 2025-05-12T14:54:09Z

Thanks for this patch. And sorry it's taken so long to review it.

I left some comments for @gergely-sallai or @tturner to address.

One question: why not just have a single toggle action, rather than separate up&down actions? Seems to add a lot of GUI clutter.

zx2c4 · 2025-05-12T14:56:58Z

ui/src/main/res/layout/tunnel_list_item.xml

+            android:onCheckedChanged="@{fragment::setShortcutState}"
+            android:tooltipText="@string/tooltip_tunnel_shortcut"
+            app:buttonTint="?attr/colorSecondary"
+            tools:checked="@sample/interface_names.json/names/shortcut/shortcut" />


I wonder if this can be more hidden or less prominent or something. This is not something most users want.

Another approach -- and the one we use on iOS -- which I think might be better is to just add toggle shortcuts for the last N used tunnels.

zx2c4-bot force-pushed the master branch from 89b0d92 to 04e503c Compare July 7, 2023 03:06

This comment was marked as off-topic.

Sign in to view

mich2k mentioned this pull request Aug 26, 2023

FR: Merging Android Dynamic Shortcuts wgtunnel/wgtunnel#21

Closed

zx2c4-bot force-pushed the master branch 4 times, most recently from 827495b to 4ba8794 Compare October 22, 2023 00:35

zx2c4-bot force-pushed the master branch 4 times, most recently from c1d59a2 to 6aab7cc Compare May 8, 2025 02:59

zx2c4 reviewed May 12, 2025

View reviewed changes

zx2c4 self-assigned this May 12, 2025

zx2c4 mentioned this pull request May 12, 2025

Add Dynamic Shortcuts integration #48

Closed

zx2c4 reviewed May 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ui: Implement support for Android Dynamic Shortcuts #55

ui: Implement support for Android Dynamic Shortcuts #55

tturner commented Jun 11, 2023

Uh oh!

This comment was marked as off-topic.

zx2c4 May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

zx2c4 commented May 12, 2025

Uh oh!

zx2c4 May 12, 2025

Uh oh!

ui: Implement support for Android Dynamic Shortcuts #55

Are you sure you want to change the base?

ui: Implement support for Android Dynamic Shortcuts #55

Conversation

tturner commented Jun 11, 2023

Uh oh!

This comment was marked as off-topic.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zx2c4 commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!