koreader / android-luajit-launcher

Android NativeActivity based launcher for LuaJIT, implementing the main loop within Lua land via FFI
MIT License
131 stars 84 forks source link

decouple dictionary actions from generic shareText #371

Closed pazos closed 2 years ago

pazos commented 2 years ago

Intended to fix https://github.com/koreader/koreader/issues/9178

Since I needed to write a new function for generic sendText I implemented @uroybd request at https://github.com/koreader/koreader/pull/9153#issuecomment-1143857139

Needs a minor change in the frontend:

diff --git a/frontend/apps/reader/modules/readerhighlight.lua b/frontend/apps/reader/modules/readerhighlight.lua
index 5f439ee6..344bdf12 100644
--- a/frontend/apps/reader/modules/readerhighlight.lua
+++ b/frontend/apps/reader/modules/readerhighlight.lua
@@ -179,7 +179,7 @@ function ReaderHighlight:init()
                     local text = cleanupSelectedText(_self.selected_text.text)
                     -- call self:onClose() before calling the android framework
                     _self:onClose()
-                    Device.doShareText(text)
+                    Device:doShareText(text)
                 end,
             }
         end)
diff --git a/frontend/device/android/device.lua b/frontend/device/android/device.lua
index 3bd181b2..c7895279 100644
--- a/frontend/device/android/device.lua
+++ b/frontend/device/android/device.lua
@@ -97,7 +97,9 @@ local Device = Generic:new{
     hasExternalSD = function() return android.getExternalSdPath() end,
     importFile = function(path) android.importFile(path) end,
     canShareText = yes,
-    doShareText = function(text) android.sendText(text) end,
+    doShareText = function(self, text, reason, title, mimetype)
+        android.sendText(text, reason or _("Share text"), title, mimetype)
+    end,

     canExternalDictLookup = yes,
     getExternalDictLookupList = function() return external.dicts end,

After that the intended way of calling the function would be something like:

Device:doShareText(text, "_(Export JSON)", "Document Title", "text/json")

The "reason" is shown on older android versions (prior to the quick share sheet feature) and ignored on newer ones. See screenshots: Screenshot_20220608_142133

Screenshot_20220608_142140


This change is Reviewable

pazos commented 2 years ago

Cool. Fixed a couple of codacy warnings too.

And bumped the LargeClass threshold.

I guess it is ok on some projects but doesn't really fit our scope here.

Classes should generally have one responsibility. Large classes can indicate that the class does instead handle multiple responsibilities. Instead of doing many things at once prefer to split up large classes into smaller classes. These smaller classes are then easier to understand and handle less things.

I wonder if there's a way to disable it for MainActivity but don't want to research ;p