Rebuild the editing UI around one tool per tap, and sell it by the tool

CLAUDE.md

@@ -119,10 +119,16 @@ Code that has to *ask* reads `Features`, never the flavor name. `Features.withAd
 from `LINKS_ADS`, which sits in `src/ads` and `src/noAds` next to the classes it stands for,
 so the flag cannot end up in a build whose code says otherwise. `Features.advancedEditing`,
 from `ADVANCED_EDITING` in the same two files, is what pro is sold on: new and joined
-paragraphs, formatting, and marks on a pdf. Every other edit the core takes - inside one
-paragraph, a sheet cell, a plain text file - is in every build. `Features.offersEditing` is the
-one list of it; the Edit button still stands on the core's answer, so over a pdf in lite it
-offers pro instead. `OpenDocument.ios` draws the same line with the same flag, beside its own
+paragraphs, formatting past the highlighter, and a pdf's marks past the highlighter. Every
+other edit the core takes - inside one paragraph, a sheet cell, a plain text file - is in every
+build.
+
+**The gate is on the tool, not on the mode.** Every edition opens every kind the core names
+editable, so `Features.offersEditing` is the core's answer alone; a locked `EditingTools` dims
+what only offers pro and leaves the **highlighter** working, under both the names it has
+(`highlight` is the formatting style and the pdf's marking tool alike). A mode nobody can do
+anything in is not worth opening, and a reader who has highlighted a paragraph knows what the
+rest would do. Do not put the whole-mode gate back. `OpenDocument.ios` draws the same line with the same flag, beside its own
 `LINKS_ADS`.
 
 Do not add a `BuildConfig.FLAVOR` comparison back - it was what made `BillingManager` miss
@@ -326,6 +332,22 @@ callbacks. Lite narrows `HtmlConfig.editingScope` to `PARAGRAPH`, and the page r
 rest with `outOfScope`, which `DocumentFragment` answers with the offer of pro, once an edit.
 A pdf needs no scaffolding: every pdf page carries `odr.annotation`.
 
+**The bar holds what is done to the document, the strip what is done to the text.** Undo, redo
+and save are `menu/edit.xml`, so they do not scroll away, and `EditActionModeCallback` dims them
+on what the page reports. `EditingTools` under the bar is the formatting alone, which is why a
+sheet or a plain text file shows no strip at all. Every tool is one 48dp square: a tap does the
+tool's one job, and a **long press** opens the colours it applies - there is no second button
+beside a tool, and the bar under its icon is what the next tap uses, not what the selection is.
+The text colour is the exception, and opens its colours on a tap too, because it has no state to
+turn off. Do not put the chevrons back, and do not move the sizes into a menu: fourteen of them
+covered the document they are about.
+
+**A pdf's tools mark what is selected; only the pen is a mode.** `odr.annotation.press` marks a
+standing selection and arms only what it could not mark, so `PageView.pressMarkTool` puts every
+tool but `ink` back down and reports what happened. With nothing selected the answer is the
+`action_annotate_banner` snackbar, not a tool left armed. The page's own `markOnSelection` is
+therefore off: it marked as the selection was *made*, which is the opposite order.
+
 **Nothing is held open between the render and the save.** `CoreLoader.writeEdits` opens the
 cached copy again and applies the page's payload with the call its kind takes -
 `Document.edit` and `save`, `TextFile.edit` and `save`, `PdfFile.annotate`. An edit that throws

app/src/androidTest/java/app/opendocument/droid/test/MainActivityTests.kt

@@ -158,17 +158,99 @@ class MainActivityTests {
         // hierarchy whether the column is open or not, and doesNotExist walks all of it
         onView(withContentDescription(R.string.menu_edit)).check(doesNotExist())
 
-        // the button is there in every edition: pro marks, lite says what pro would do
+        // every edition marks up a pdf; what lite does not sell is the tool, and one of them
+        // is free so that the mode is worth opening
         onView(withContentDescription(R.string.menu_annotate)).perform(click())
 
-        if (Features.advancedEditing) {
-            onView(withContentDescription(R.string.tool_mark_draw)).check(matches(isDisplayed()))
-        } else {
+        onView(withContentDescription(R.string.tool_mark_draw)).check(matches(isDisplayed()))
+        onView(withContentDescription(R.string.tool_mark_highlight)).check(matches(isDisplayed()))
+
+        if (!Features.advancedEditing) {
+            // the pen is pro's, and says so rather than arming itself
+            onView(withContentDescription(R.string.tool_mark_draw)).perform(click())
+
             awaitViewWithText(R.string.pro_offer_title)
             onView(withText(R.string.pro_offer_title)).check(matches(isDisplayed()))
         }
     }
 
+    /**
+     * A pdf's tools mark the text that is selected and are then put down again. Only the pen stays
+     * armed, because a stroke has no selection to go on - and only where this edition sells it: the
+     * highlighter is the free one, and marks in lite too.
+     */
+    @Test
+    fun onlyThePenStaysArmed() {
+        val activity = mainActivityActivityTestRule.activity
+        val documentFragment = loadDocument(activity, requireTestFile("dummy.pdf"))
+        val pageView = requireNotNull(documentFragment.pageView)
+
+        enterEditMode(activity, documentFragment)
+
+        Assert.assertTrue(
+            "the page should carry the annotator",
+            waitFor(EDIT_MODE_TIMEOUT_MS) { pageAnswers(pageView, "odr.annotation") },
+        )
+
+        val selected = AtomicReference("")
+        Assert.assertTrue(
+            "the page should have text to select, and answered '${'$'}{selected.get()}'",
+            waitFor(EDIT_MODE_TIMEOUT_MS) {
+                selected.set(selectPageText(pageView))
+
+                selected.get().isNotEmpty()
+            },
+        )
+
+        onView(withContentDescription(R.string.tool_mark_highlight)).perform(click())
+
+        Assert.assertTrue(
+            "the selection should be marked",
+            waitFor(EDIT_MODE_TIMEOUT_MS) {
+                pageAnswers(pageView, "odr.annotation.list().length === 1")
+            },
+        )
+        Assert.assertTrue(
+            "the tool should not stay armed",
+            pageAnswers(pageView, "odr.annotation.getTool() === null"),
+        )
+
+        // the pen is the one that does, and it marks nothing by being pressed. In lite it is
+        // pro's and says so instead, the highlighter above being the free one
+        if (!Features.advancedEditing) {
+            return
+        }
+
+        onView(withContentDescription(R.string.tool_mark_draw)).perform(click())
+
+        Assert.assertTrue(
+            "the pen should stay armed",
+            waitFor(EDIT_MODE_TIMEOUT_MS) {
+                pageAnswers(pageView, "odr.annotation.getTool() === 'ink'")
+            },
+        )
+        Assert.assertTrue(
+            "the pen should have marked nothing",
+            pageAnswers(pageView, "odr.annotation.list().length === 1"),
+        )
+    }
+
+    /** Selects everything the page holds, and answers what that turned out to be. */
+    private fun selectPageText(pageView: PageView): String =
+        evaluateJavascript(
+                pageView,
+                "(function(){" +
+                    "var range = document.createRange();" +
+                    "range.selectNodeContents(document.body);" +
+                    "var selection = window.getSelection();" +
+                    "selection.removeAllRanges();" +
+                    "selection.addRange(range);" +
+                    "return selection.toString().trim();" +
+                    "})()",
+            )
+            ?.replace("\"", "")
+            .orEmpty()
+
     /** A sheet takes cell edits in every edition. */
     @Test
     fun aSheetIsEditedInEveryEdition() {

app/src/main/assets/editing-bridge.js

@@ -27,9 +27,9 @@
     bridge.marksChanged(event.count);
   };
 
-  if (odr.annotation) {
-    // an armed tool marks a selection as it is made, which is what a touch screen needs: with a
-    // selection standing, the selection's own toolbar is over the page
-    odr.annotation.setOptions({ markOnSelection: true });
+  if (odr.editing && odr.editing.setSheetOptions) {
+    // a phone has no double click to spare, and the pointer is not asked: a
+    // WebView on an emulator answers that one as a mouse
+    odr.editing.setSheetOptions({ editOnClick: true });
   }
 })();

app/src/main/java/app/opendocument/droid/nonfree/Features.kt

@@ -18,7 +18,10 @@ object Features {
     /** Formatting, new and joined paragraphs, and pdf marks: pro and foss. */
     val advancedEditing = ADVANCED_EDITING
 
-    /** Whether this build opens the edit mode for [kind], which the core decided. */
-    fun offersEditing(kind: EditingKind): Boolean =
-        kind.isEditable && (kind != EditingKind.ANNOTATION || advancedEditing)
+    /**
+     * Whether this build opens the edit mode for [kind], which the core decided. Every edition
+     * opens every kind it can: what lite does not sell is the individual tool, which carries pro's
+     * badge in the strip, and one of each kind is free so that the mode is worth opening.
+     */
+    fun offersEditing(kind: EditingKind): Boolean = kind.isEditable
 }

app/src/main/java/app/opendocument/droid/ui/EditActionModeCallback.kt

@@ -2,51 +2,107 @@ package app.opendocument.droid.ui
 
 import android.view.Menu
 import android.view.MenuItem
-import android.widget.TextView
 import androidx.appcompat.view.ActionMode
 import app.opendocument.droid.R
 import app.opendocument.droid.background.EditingKind
 import app.opendocument.droid.ui.activity.DocumentFragment
 import app.opendocument.droid.ui.activity.MainActivity
 
-/** The edit mode: the bar with save. The tools under it are `EditingTools`. */
+/**
+ * The edit mode: the bar with undo, redo and save. The tools that change the text itself are
+ * `EditingTools`, in the strip under the bar.
+ */
 class EditActionModeCallback(
     private val activity: MainActivity,
     private val documentFragment: DocumentFragment,
 ) : ActionMode.Callback {
 
+    private var annotating = false
+
+    private var undoItem: MenuItem? = null
+    private var redoItem: MenuItem? = null
+
+    private var canUndo = false
+    private var canRedo = false
+
     override fun onCreateActionMode(mode: ActionMode, menu: Menu): Boolean {
-        val annotating = documentFragment.editingKind == EditingKind.ANNOTATION
+        annotating = documentFragment.editingKind == EditingKind.ANNOTATION
 
-        val statusView = TextView(activity)
-        statusView.setText(
-            if (annotating) R.string.action_annotate_banner else R.string.action_edit_banner
-        )
-        mode.customView = statusView
+        // the name of the mode, not a sentence: three buttons leave a phone's bar no room for one
+        mode.setTitle(if (annotating) R.string.menu_annotate else R.string.menu_edit)
 
         mode.menuInflater.inflate(R.menu.edit, menu)
 
+        undoItem = menu.findItem(R.id.edit_undo)
+        redoItem = menu.findItem(R.id.edit_redo)
+        redoItem?.isVisible = !annotating
+
+        setUndoState(canUndo = false, canRedo = false)
+
         documentFragment.setEditing(true)
 
+        if (annotating) {
+            // what the strip cannot say: a mark goes on the text that is selected
+            SnackbarHelper.show(
+                activity,
+                R.string.action_annotate_banner,
+                null,
+                isIndefinite = false,
+                isError = false,
+            )
+        }
+
         return true
     }
 
     override fun onPrepareActionMode(mode: ActionMode, menu: Menu): Boolean = false
 
+    /** What the page says can be taken back and put back. */
+    fun setUndoState(canUndo: Boolean, canRedo: Boolean) {
+        this.canUndo = canUndo
+        this.canRedo = canRedo
+
+        undoItem?.let { setUsable(it, canUndo) }
+        redoItem?.let { setUsable(it, canRedo) }
+    }
+
+    /** A bar item is not dimmed by being disabled, so its icon is dimmed here. */
+    private fun setUsable(item: MenuItem, usable: Boolean) {
+        item.isEnabled = usable
+        item.icon?.alpha = if (usable) OPAQUE else DISABLED_ALPHA
+    }
+
     override fun onActionItemClicked(mode: ActionMode, item: MenuItem): Boolean {
-        if (item.itemId != R.id.edit_save) {
-            return false
-        }
+        when (item.itemId) {
+            R.id.edit_undo -> {
+                activity.analyticsManager.report("menu_edit_undo")
+
+                documentFragment.undo()
+            }
+
+            R.id.edit_redo -> {
+                activity.analyticsManager.report("menu_edit_redo")
 
-        // OpenDocument.ios' name for this; menu_save is the button on the document itself
-        activity.analyticsManager.report("menu_edit_save")
+                documentFragment.redo()
+            }
 
-        documentFragment.prepareSave({ activity.requestSave() }, false)
+            R.id.edit_save -> {
+                // OpenDocument.ios' name for this; menu_save is the button on the document itself
+                activity.analyticsManager.report("menu_edit_save")
+
+                documentFragment.prepareSave({ activity.requestSave() }, false)
+            }
+
+            else -> return false
+        }
 
         return true
     }
 
     override fun onDestroyActionMode(mode: ActionMode) {
+        undoItem = null
+        redoItem = null
+
         documentFragment.setEditing(false)
 
         // the page keeps its edits with the mode off. not when the document is being closed: that
@@ -55,4 +111,12 @@ class EditActionModeCallback(
             activity.confirmLeavingEdits { documentFragment.discardEdits() }
         }
     }
+
+    companion object {
+
+        private const val OPAQUE = 255
+
+        /** Material's opacity for a disabled icon, 38%. */
+        private const val DISABLED_ALPHA = 97
+    }
 }