Menu now shows all 3 elements

app/src/main/AndroidManifest.xml

@@ -27,6 +27,5 @@
                 <category android:name="android.intent.category.LAUNCHER" />
             </intent-filter>
         </activity>
-
     </application>
 </manifest>

app/src/main/kotlin/org/bspeice/minimalbible/activity/viewer/BibleMenu.kt

@@ -12,7 +12,6 @@ import org.bspeice.minimalbible.R
 import android.content.Context
 import android.view.LayoutInflater
 import android.content.res.Resources
-import android.support.annotation.IdRes
 import android.widget.ExpandableListView
 import rx.subjects.PublishSubject
 import android.widget.LinearLayout
@@ -29,7 +28,7 @@ class BibleMenu(val ctx: Context, val attrs: AttributeSet) : LinearLayout(ctx, a
         val inflater = ctx.getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater
         inflater.inflate(R.layout.view_bible_menu, this, true)
 
-        menuContent = findViewById(R.id.menu) as ExpandableListView
+        menuContent = findViewById(R.id._bible_menu) as ExpandableListView
     }
 
     fun setBible(b: Book) = menuContent.setAdapter(BibleAdapter(b))
@@ -37,6 +36,18 @@ class BibleMenu(val ctx: Context, val attrs: AttributeSet) : LinearLayout(ctx, a
     fun placeInset(a: Activity) = setInset(a)
 }
 
+/**
+ * The actual adapter for displaying a book's menu navigation system.
+ * There are a couple of notes about this:
+ *  Books are displayed with one row per BibleBook (Genesis, Exodus, etc.) as the group.
+ *  Within each group, there are 3 chapters listed per row (to save space). In order to
+ *  accommodate this, some slightly funky mathematics have to be used, and this is documented.
+ *  Additionally, it doesn't make a whole lot of sense to genericize this using constants
+ *  unless we go to programmatic layouts, since we still need to know the view ID's ahead of time.
+ *
+ * TODO: Refactor this so the math parts are separate from the actual override functions,
+ * so it's easier to test.
+ */
 class BibleAdapter(val b: Book) : BaseExpandableListAdapter() {
 
     // Map BibleBooks to the number of chapters they have
@@ -67,11 +78,30 @@ class BibleAdapter(val b: Book) : BaseExpandableListAdapter() {
 
     override fun getGroupCount(): Int = menuMappings.count()
 
-    override fun getChildrenCount(group: Int): Int = menuMappings[group].second
+    fun getChaptersForGroup(group: Int) = menuMappings[group].second
+
+    /**
+     * Get the number of child views for a given book.
+     * What makes this complicated is that we display 3 chapters per row.
+     * To make sure we include everything and account for integer division,
+     * we have to add a row if the chapter count modulo 3 is not even.
+     */
+    override fun getChildrenCount(group: Int): Int {
+        val chapterCount = getChaptersForGroup(group)
+        return when (chapterCount % 3) {
+            0 -> chapterCount / 3
+            else -> (chapterCount / 3) + 1
+        }
+    }
 
     override fun getGroup(group: Int): String = b.bookName(menuMappings[group].first)
 
-    override fun getChild(group: Int, child: Int): Int = child + 1 // Index offset
+    /**
+     * Get the starting chapter number for this child view
+     * In order to account for displaying 3 chapters per line,
+     * we need to multiply by three, and then add 1 for the index offset
+     */
+    override fun getChild(group: Int, child: Int): Int = (child * 3) + 1
 
     override fun getGroupId(group: Int): Long = group.toLong()
 
@@ -81,38 +111,51 @@ class BibleAdapter(val b: Book) : BaseExpandableListAdapter() {
 
     override fun isChildSelectable(group: Int, child: Int): Boolean = true
 
-    private fun doBinding(convertView: View?, parent: ViewGroup,
-                          obj: Any, highlight: Boolean,
-                          LayoutRes layout: Int): View {
-        val finalView: View = convertView ?:
-                (parent.getContext()
-                        .getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater)
-                        .inflate(layout, parent, false)
-
-        val holder: NavItemHolder =
-                if (finalView.getTag() != null) finalView.getTag() as NavItemHolder
-                else NavItemHolder(finalView, R.id.content)
-
-        holder.bind(obj, highlight)
-        finalView setTag holder
-        return finalView
-    }
-
     override fun getGroupView(position: Int, expanded: Boolean,
                               convertView: View?, parent: ViewGroup): View =
-            doBinding(convertView, parent, getGroup(position),
+            GroupItemHolder.init(
-                    position == groupHighlighted, R.layout.list_bible_menu_group)
+                    getOrInflate(convertView, parent, R.layout.list_bible_menu_group),
+                    getGroup(position),
+                    position == groupHighlighted)
 
     override fun getChildView(group: Int, child: Int, isLast: Boolean,
-                              convertView: View?, parent: ViewGroup): View =
+                              convertView: View?, parent: ViewGroup): View {
-            doBinding(convertView, parent, getChild(group, child),
+        val chapterStart = getChild(group, child)
-                    group == groupHighlighted && child == childHighlighted,
+        val chapterCount = getChaptersForGroup(group)
-                    R.layout.list_bible_menu_child)
+        val chapterEnd =
+                if (chapterCount < chapterStart + 2)
+                    chapterCount
+                else
+                    chapterStart + 2
+        val view = ChildItemHolder.init(
+                getOrInflate(convertView, parent, R.layout.list_bible_menu_child),
+                chapterStart..chapterEnd
+        )
 
-    class NavItemHolder(val bindTo: View, IdRes resource: Int) {
+        return view
-        val content = bindTo.findViewById(resource) as TextView
+    }
+
+    private fun getOrInflate(v: View?, p: ViewGroup, LayoutRes layout: Int) =
+            v ?: (p.getContext()
+                    .getSystemService(Context.LAYOUT_INFLATER_SERVICE) as LayoutInflater)
+                    .inflate(layout, p, false)
+}
+
+class GroupItemHolder(val bindTo: View) {
+    val content = bindTo.findViewById(R.id.content) as TextView
     val resources = bindTo.getResources(): Resources
 
+    class object {
+        fun init(v: View, obj: Any, highlighted: Boolean): View {
+            val holder =
+                    if (v.getTag() != null) v.getTag() as GroupItemHolder
+                    else GroupItemHolder(v)
+            holder.bind(obj, highlighted)
+
+            return v
+        }
+    }
+
     fun getHighlightedColor(highlighted: Boolean) =
             if (highlighted) resources getColor R.color.colorAccent
             else resources getColor R.color.textColor
@@ -121,5 +164,53 @@ class BibleAdapter(val b: Book) : BaseExpandableListAdapter() {
         content setText obj.toString()
         content setTextColor getHighlightedColor(highlighted)
     }
+}
+
+/**
+ * Bind the child items. There are some funky math things going on since
+ * we display three chapters per row, check the adapter for more documentation
+ */
+class ChildItemHolder(val bindTo: View) {
+    val content1 = bindTo.findViewById(R.id.content1) as TextView
+    val content2 = bindTo.findViewById(R.id.content2) as TextView
+    val content3 = bindTo.findViewById(R.id.content3) as TextView
+
+    class object {
+        fun init(v: View, obj: IntRange): View {
+            val holder =
+                    if (v.getTag() != null) v.getTag() as ChildItemHolder
+                    else ChildItemHolder(v)
+
+            holder.clearViews()
+            holder.bind(obj)
+            return v
+        }
+    }
+
+    // Clear the views before binding, so that we don't have stale text left
+    // as a result of recycling. There should probably be a different way of doing this,
+    // but get something that works first.
+    fun clearViews() {
+        content1 setText ""
+        content2 setText ""
+        content3 setText ""
+    }
+
+    /**
+     * Calculate which view should hold the chapter. We remove 1 before the modulus
+     * in order to use index-based addressing. If we didn't remove 1, position 1 would receive
+     * content2, since 1 modulus 3 is 1.
+     */
+    fun getViewForPosition(position: Int) = when ((position - 1) % 3) {
+        0 -> content1
+        1 -> content2
+        else -> content3
+    }
+
+    /**
+     * Set up the view with the data we want to display
+     */
+    fun bind(range: IntRange) = range.forEach {
+        getViewForPosition(it) setText it.toString()
     }
 }

app/src/main/res/layout/list_bible_menu_child.xml

@@ -1,17 +1,50 @@
 <?xml version="1.0" encoding="utf-8"?>
+
+<!--
+Layout for displaying child elements of the Bible Menu
+This needs a bit of explaining since its a bit complicated.
+There are three TextViews, each for displaying a single chapter.
+In order to make sure they are all aligned correctly, *even when
+one or more doesn't have any text*, they are set to 0dp width initially,
+and the weights are used to determine how big they should actually be.
+This way, no "wrap_content" width is used, messing up alignment
+because one cell doesn't have a value.
+-->
 <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
-    android:orientation="vertical">
+    android:orientation="horizontal">
 
     <TextView
-        android:id="@+id/content"
+        android:id="@+id/content1"
-        android:layout_width="match_parent"
+        android:layout_width="0dp"
         android:layout_height="wrap_content"
-        android:gravity="center_vertical"
+        android:gravity="center_vertical|center_horizontal"
         android:minHeight="?android:attr/listPreferredItemHeight"
-        android:paddingLeft="?android:attr/expandableListPreferredChildPaddingLeft"
+        android:paddingLeft="@dimen/biblemenu_child_padding"
-        android:paddingRight="?android:attr/expandableListPreferredChildPaddingLeft"
+        android:paddingRight="@dimen/biblemenu_child_padding"
-        android:textAppearance="?android:attr/textAppearanceMedium" />
+        android:textAppearance="?android:attr/textAppearanceMedium"
+        android:layout_weight="1" />
 
+    <TextView
+        android:id="@+id/content2"
+        android:layout_width="0dp"
+        android:layout_height="wrap_content"
+        android:gravity="center_vertical|center_horizontal"
+        android:minHeight="?android:attr/listPreferredItemHeight"
+        android:paddingLeft="@dimen/biblemenu_child_padding"
+        android:paddingRight="@dimen/biblemenu_child_padding"
+        android:textAppearance="?android:attr/textAppearanceMedium"
+        android:layout_weight="1" />
+
+    <TextView
+        android:id="@+id/content3"
+        android:layout_width="0dp"
+        android:layout_height="wrap_content"
+        android:gravity="center_vertical|center_horizontal"
+        android:minHeight="?android:attr/listPreferredItemHeight"
+        android:paddingLeft="@dimen/biblemenu_child_padding"
+        android:paddingRight="@dimen/biblemenu_child_padding"
+        android:textAppearance="?android:attr/textAppearanceMedium"
+        android:layout_weight="1" />
 </LinearLayout>

app/src/main/res/layout/view_bible_menu.xml

@@ -8,14 +8,14 @@
     <!-- Both paddingLeft and Right are given to support RtL
     layouts without worrying about min API and paddingStart shenanigans -->
     <TextView
-        android:id="@+id/title"
+        android:id="@+id/_bible_title"
         style="@style/MinimalBible.NavigationDrawer.Title"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
         android:text="@string/app_name" />
 
     <ExpandableListView
-        android:id="@+id/menu"
+        android:id="@+id/_bible_menu"
         android:layout_width="match_parent"
         android:layout_height="wrap_content" />
 

app/src/main/res/values/dimens.xml

@@ -9,4 +9,6 @@
     <dimen name="navigation_drawer_highlight_height">32dp</dimen>
 
     <dimen name="toolbar_height">56dp</dimen>
+
+    <dimen name="biblemenu_child_padding">8dp</dimen>
 </resources>

app/src/main/res/values/strings.xml

@@ -2,14 +2,10 @@
 <resources>
 
     <string name="app_name">MinimalBible</string>
-    <string name="title_section1">Section 1</string>
-    <string name="title_section2">Section 2</string>
-    <string name="title_section3">Section 3</string>
     <string name="navigation_drawer_open">Open navigation drawer</string>
     <string name="navigation_drawer_close">Close navigation drawer</string>
     <string name="action_settings">Settings</string>
     <string name="activity_downloader">Downloads</string>
-
     <string name="book_removal_failure">Could not remove book. Try restarting application?</string>
     <string name="action_download_title_categories">Categories</string>