Merge pull request #1118 from wordpress-mobile/dodroid-954-checklist-issue

Fix checkbox interaction for indented (nested) task list items

Author: zwarm

aztec/src/main/kotlin/org/wordpress/aztec/AztecText.kt

@@ -751,16 +751,11 @@ open class AztecText : AppCompatEditText, TextWatcher, UnknownHtmlSpan.OnUnknown
         x += scrollX
         y += scrollY
 
-        // We check whether the user tap on a checkbox of a task list. The aztec text field should be enabled and we
-        // check whether the tap event was on the leading margin which is where the checkbox is located plus a padding
-        // space used to increase the target size for the tap event.
-        if (isEnabled &&
-            x + totalPaddingStart <= (blockFormatter.listStyleLeadingMargin() + AztecTaskListSpan.PADDING_SPACE)) {
+        // We check whether the user tap on a checkbox of a task list. The handler performs its
+        // own nesting-aware x-coordinate check, so we only need a rough gate here.
+        if (isEnabled) {
             val line = layout.getLineForVertical(y)
             val off = layout.getOffsetForHorizontal(line, x.toFloat())
-            // If the tap event was on the leading margin, we double check whether we are tapping on a task list item.
-            // If that is true, then we return false because we don't want to propagate the tap event to stop moving
-            // the cursor to the item tapped.
             if (getTaskListHandler()?.handleTaskListClick(
                 text,
                 off,

aztec/src/main/kotlin/org/wordpress/aztec/TaskListClickHandler.kt

@@ -7,15 +7,18 @@ import org.wordpress.aztec.spans.AztecTaskListSpan
 
 class TaskListClickHandler(val listStyle: BlockFormatter.ListStyle) {
     fun handleTaskListClick(text: Spannable, off: Int, x: Int, startMargin: Int): Boolean {
-        // We want to make sure that text click will not trigger the checked change
-        if (x + startMargin > (listStyle.leadingMargin() + AztecTaskListSpan.PADDING_SPACE)) return false
-        val clickedList = text.getSpans(off, off, AztecTaskListSpan::class.java).firstOrNull()
+        val clickedList = text.getSpans(off, off, AztecTaskListSpan::class.java).maxByOrNull { it.nestingLevel }
+                ?: return false
+        // Account for nested leading margins: each nesting level adds one leadingMargin width
+        val depthMultiplier = (clickedList.nestingLevel / 2) + 1
+        val effectiveMargin = listStyle.leadingMargin() * depthMultiplier
+        if (x + startMargin > (effectiveMargin + AztecTaskListSpan.PADDING_SPACE)) return false
         val clickedLines = text.getSpans(off, off, AztecListItemSpan::class.java)
         val clickedLine = clickedLines.find {
             val spanStart = text.getSpanStart(it)
             spanStart == off || (spanStart == off - 1 && text.getSpanEnd(it) == off)
         }
-        if (clickedList != null && clickedLine != null && clickedList.canToggle()) {
+        if (clickedLine != null && clickedList.canToggle()) {
             clickedLine.toggleCheck()
             clickedList.refresh()
             return true

aztec/src/main/kotlin/org/wordpress/aztec/spans/AztecTaskListSpan.kt

@@ -139,9 +139,9 @@ open class AztecTaskListSpan(
     private fun isChecked(text: CharSequence, lineIndex: Int): Boolean {
         val spanStart = (text as Spanned).getSpanStart(this)
         val spanEnd = text.getSpanEnd(this)
-        val sortedSpans = text.getSpans(spanStart, spanEnd, AztecListItemSpan::class.java).sortedBy {
-            text.getSpanStart(it)
-        }
+        val sortedSpans = text.getSpans(spanStart, spanEnd, AztecListItemSpan::class.java)
+                .filter { it.nestingLevel == nestingLevel + 1 }
+                .sortedBy { text.getSpanStart(it) }
         return sortedSpans.getOrNull(lineIndex - 1)?.attributes?.getValue("checked") == "true"
     }