Skip to content

Commit

Permalink
Logic simplifications
Browse files Browse the repository at this point in the history
Signed-off-by: Danny Baumann <[email protected]>
  • Loading branch information
maniac103 committed Jul 5, 2024
1 parent b31f58f commit dfb20a0
Showing 1 changed file with 30 additions and 44 deletions.
74 changes: 30 additions & 44 deletions mobile/src/main/java/org/openhab/habdroid/ui/WidgetAdapter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ class WidgetAdapter(
val itemList: List<Widget> get() = items
private val widgetsById = mutableMapOf<String, Widget>()
private val widgetsByParentId = mutableMapOf<String, MutableList<Widget>>()
val widgetsByParentIdMap: Map<String, List<Widget>> get() = widgetsByParentId
val hasVisibleWidgets: Boolean
get() = items.any { widget -> shouldShowWidget(widget) }

Expand Down Expand Up @@ -172,9 +171,9 @@ class WidgetAdapter(
widgetsByParentId.clear()
widgets.forEach { w ->
widgetsById[w.id] = w
w.parentId?.let { parentId ->
widgetsByParentId.getOrPut(parentId) { mutableListOf() }.add(w)
}
w.parentId
?.let { parentId -> widgetsByParentId.getOrPut(parentId) { mutableListOf() } }
?.add(w)
}
notifyDataSetChanged()
}
Expand Down Expand Up @@ -239,7 +238,7 @@ class WidgetAdapter(
TYPE_LOCATION -> MapViewHelper.createViewHolder(initData)
TYPE_INPUT -> InputViewHolder(initData)
TYPE_DATETIMEINPUT -> DateTimeInputViewHolder(initData)
TYPE_BUTTONGRID -> ButtongridViewHolder(initData)
TYPE_BUTTONGRID -> ButtongridViewHolder(initData) { id -> widgetsByParentId[id] }
TYPE_INVISIBLE -> InvisibleWidgetViewHolder(initData)
else -> throw IllegalArgumentException("View type $viewType is not known")
}
Expand Down Expand Up @@ -305,24 +304,20 @@ class WidgetAdapter(
val oldWidget = items[position]
items[position] = widget
widgetsById[widget.id] = widget
widgetsByParentId[oldWidget.parentId]?.let {
it.remove(oldWidget)
if (it.isEmpty()) {
widgetsByParentId.remove(oldWidget.parentId)
}
}
widget.parentId?.let { parentId ->
widgetsByParentId.getOrPut(parentId) { mutableListOf() }.add(widget)
}
widgetsByParentId[oldWidget.parentId]?.remove(oldWidget)
widget.parentId
?.let { parentId -> widgetsByParentId.getOrPut(parentId) { mutableListOf() } }
?.add(widget)
// If visibility of a container with at least one child changes, refresh the whole list to make sure
// the child visibility is also updated. Otherwise it's sufficient to update the single widget only.
if (oldWidget.visibility != widget.visibility && items.any { w -> w.parentId == widget.id }) {
notifyDataSetChanged()
} else {
// update the parent Buttongrid if the updated widget is a button
if (widget.type == Widget.Type.Button && widget.parentId != null) {
items.indexOfFirst { w -> w.id == widget.parentId }?.let { position ->

This comment has been minimized.

Copy link
@maniac103

maniac103 Jul 5, 2024

Author Owner

This was an actual bug: indexOfFirst() never returns null; it returns -1 if the item can not be found.

This comment has been minimized.

Copy link
@jimtng

jimtng Jul 5, 2024

rookie mistake! :)

This comment has been minimized.

Copy link
@jimtng

jimtng Jul 5, 2024

I've seen people use .takeIf { it >= 0 }?.let ...

This comment has been minimized.

Copy link
@maniac103

maniac103 Jul 5, 2024

Author Owner

Yes, this also works:

                items
                    .indexOfFirst { w -> w.id == widget.parentId }
                    .takeIf { it >= 0 }
                    ?.let { notifyItemChanged(it) }

Not sure whether it's more elegant/readable though.

This comment has been minimized.

Copy link
@jimtng

jimtng Jul 5, 2024

yeah either way is fine, no need to change it. Sometimes the functional style can get quite convoluted and became harder to read too, as evidenced by my button.stateless?.not() :? xxx code that you simplified, lol!

notifyItemChanged(position)
val parentPosition = items.indexOfFirst { w -> w.id == widget.parentId }
if (parentPosition >= 0) {
notifyItemChanged(parentPosition)
}
} else {
notifyItemChanged(position)
Expand Down Expand Up @@ -839,13 +834,15 @@ class WidgetAdapter(
}
}

class ButtongridViewHolder internal constructor(private val initData: ViewHolderInitData) :
LabeledItemBaseViewHolder(initData, R.layout.widgetlist_buttongriditem),
class ButtongridViewHolder internal constructor(
private val initData: ViewHolderInitData,
private val childWidgetCallback: (String) -> List<Widget>?
) : LabeledItemBaseViewHolder(initData, R.layout.widgetlist_buttongriditem),
View.OnClickListener,
View.OnTouchListener {
private val table: GridLayout = itemView.findViewById(R.id.widget_content)
private val maxColumns = itemView.resources.getInteger(R.integer.section_switch_max_buttons)
private val buttonViews = mutableMapOf<Pair<Int, Int>, MaterialButton>()
private val spareViews = mutableListOf<MaterialButton>()

override fun bind(widget: Widget) {
super.bind(widget)
Expand All @@ -855,39 +852,21 @@ class WidgetAdapter(
labelView.isVisible = showLabelAndIcon
iconView.isVisible = showLabelAndIcon

val buttons =
widget.mappings.mapIndexed { index, it -> it.toWidget("$widget.id-mappings-$index", widget.item) } +
(bindingAdapter as WidgetAdapter).widgetsByParentIdMap[widget.id].orEmpty()
val buttons = childWidgetCallback(widget.id).orEmpty() +
widget.mappings.mapIndexed { index, it -> it.toWidget("$widget.id-mappings-$index", widget.item) }

val rowCount = buttons.maxOfOrNull { it.row ?: 0 } ?: 0
val columnCount = min(buttons.maxOfOrNull { it.column ?: 0 } ?: 0, maxColumns)
buttonViews.filter { (position, buttonView) ->
position.first >= rowCount ||
position.second >= columnCount ||
(buttonView.tag as? Widget)?.parentId != widget.id
}
.forEach { (position, buttonView) ->
table.removeView(buttonView)
buttonViews.remove(position)
}
spareViews.addAll(table.children.map { it as? MaterialButton }.filterNotNull())
table.removeAllViews()

table.rowCount = rowCount
table.columnCount = columnCount
(0 until table.rowCount).forEach { row ->
(0 until table.columnCount).forEach { column ->
val position = Pair(row, column)
val buttonView = buttonViews.getOrPut(position) {
val newButton = initData.inflater.inflate(R.layout.widgetlist_sectionswitchitem_button, table, false)
val buttonView = spareViews.removeFirstOrNull()
?: initData.inflater.inflate(R.layout.widgetlist_sectionswitchitem_button, table, false)
as MaterialButton
table.addView(
newButton,
GridLayout.LayoutParams(
GridLayout.spec(row, GridLayout.FILL, 1f),
GridLayout.spec(column, GridLayout.FILL, 1f)
)
)
newButton
}

// Rows and columns start with 1 in Sitemap definition, thus decrement them here
val button = buttons.firstOrNull { (it.row ?: 0) - 1 == row && (it.column ?: 0) - 1 == column }
Expand All @@ -905,11 +884,18 @@ class WidgetAdapter(
iconColor = button.iconColor,
mapper = colorMapper
)
buttonView.isCheckable = button.stateless?.not() ?: false
buttonView.isCheckable = button.stateless == true
buttonView.isChecked = button.item?.state?.asString == button.command
buttonView.visibility = View.VISIBLE
}
buttonView.maxWidth = table.width / table.columnCount
table.addView(
buttonView,
GridLayout.LayoutParams(
GridLayout.spec(row, GridLayout.FILL, 1f),
GridLayout.spec(column, GridLayout.FILL, 1f)
)
)
}
}
}
Expand Down

0 comments on commit dfb20a0

Please sign in to comment.