-
Notifications
You must be signed in to change notification settings - Fork 3
feat(native-list): apply the row style on iOS and Android #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: codex/native-list-row-style
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -523,6 +523,8 @@ internal class NativeListRowView( | |
| private var marketTouchX = 0f | ||
| private var marketTouchY = 0f | ||
| private var marketLongPressFired = false | ||
| // OneKey patch: a row style wrote view properties that resetViews does not restore. | ||
| private var styledViewsDirty = false | ||
| private var reorderActive = false | ||
| private var checkboxCheckedColor = Color.rgb(32, 32, 32) | ||
| private var checkboxUncheckedColor = Color.rgb(252, 252, 252) | ||
|
|
@@ -1023,10 +1025,84 @@ internal class NativeListRowView( | |
| TextViewCompat.setLineHeight(subtitle, dp(20)) | ||
| minimumHeight = 0 | ||
| } | ||
| applyRowStyle(item) | ||
| applyListOrientation(item, listOrientation) | ||
| applySelectorTypography(item) | ||
| } | ||
|
|
||
| /** | ||
| * docs/STYLE_SPEC.md section 8. Must run after applySize(), which re-dispatches | ||
| * font size and typeface by row type and would otherwise overwrite the style. | ||
| * A style key names a model field; nativeListStyleSlot maps it to the view that | ||
| * renders that field, because the view pool is shared across templates. | ||
| */ | ||
| private fun applyRowStyle(item: NativeListItem) { | ||
| // Market owns its own richer style path, applied inside bindMarket. | ||
| if (item.type == "market") return | ||
| val style = item.json.optJSONObject("style") ?: return | ||
| styledViewsDirty = true | ||
| if (style.has("horizontalPadding")) { | ||
| val inset = dp(style.optDouble("horizontalPadding").roundToInt()) | ||
| setPadding(inset, paddingTop, inset, paddingBottom) | ||
| } | ||
| if (style.has("verticalPadding")) { | ||
| val inset = dp(style.optDouble("verticalPadding").roundToInt()) | ||
| setPadding(paddingLeft, inset, paddingRight, inset) | ||
| } | ||
| if (style.has("lineGap")) { | ||
| (subtitle.layoutParams as? MarginLayoutParams)?.topMargin = | ||
| dp(style.optDouble("lineGap").roundToInt()) | ||
| } | ||
| val variant = item.json.optString("variant") | ||
| val fields = style.keys() | ||
| while (fields.hasNext()) { | ||
| val field = fields.next() | ||
| val slotStyle = style.optJSONObject(field) ?: continue | ||
| when (val slot = nativeListStyleSlot(item.type, variant, field)) { | ||
| null -> continue | ||
| "dataPrimary" -> dataColumns.forEach { applyStyledText(it, slotStyle) } | ||
| else -> styledSlotView(slot)?.let { applyStyledText(it, slotStyle) } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun styledSlotView(slot: String): TextView? = when (slot) { | ||
| "title" -> title | ||
| "subtitle" -> subtitle | ||
| "tertiary" -> tertiary | ||
| "status" -> status | ||
| "metricSubtitle" -> metricSubtitle | ||
| "badge" -> badgeLine | ||
| "mediaBadge" -> mediaBadge | ||
| "value" -> trailingViews[0] | ||
| "valueSecondary" -> trailingViews[1] | ||
| else -> null | ||
| } | ||
|
|
||
| private fun applyStyledText(view: TextView, style: JSONObject) { | ||
| if (style.has("fontSize")) view.textSize = sp(style.optDouble("fontSize").toFloat()) | ||
| style.optString("fontWeight").takeIf(String::isNotEmpty)?.let { | ||
| view.typeface = marketTypeface(it, "regular") | ||
| } | ||
| style.optString("color").takeIf(String::isNotEmpty)?.let { | ||
| view.setTextColor(safeColor(it, view.currentTextColor)) | ||
| } | ||
| if (style.has("lineHeight")) { | ||
| TextViewCompat.setLineHeight(view, dp(style.optDouble("lineHeight").roundToInt())) | ||
| } | ||
| if (style.has("lines")) { | ||
| view.maxLines = style.optInt("lines").coerceIn(1, 2) | ||
| view.ellipsize = TextUtils.TruncateAt.END | ||
| } | ||
| style.optString("alignment").takeIf(String::isNotEmpty)?.let { | ||
| view.gravity = (view.gravity and Gravity.VERTICAL_GRAVITY_MASK) or when (it) { | ||
| "center" -> Gravity.CENTER_HORIZONTAL | ||
| "end" -> Gravity.END | ||
| else -> Gravity.START | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // OneKey patch: keep a small idle member pool after reuse or a direct rebind. | ||
| // The compact proxy/expansion keeps every member until it leaves that state. | ||
| private fun trimWalletGroupRows(required: Int) { | ||
|
|
@@ -1209,7 +1285,33 @@ internal class NativeListRowView( | |
| bindingEpoch += 1 | ||
| } | ||
|
|
||
| /** | ||
| * docs/STYLE_SPEC.md section 7 rule 2: resetViews() restores visibility, gravity, | ||
| * maxLines, layout params, background and padding - but not textSize, typeface or | ||
| * lineHeight, which each binding path re-establishes for itself. A style that wrote | ||
| * one of those would therefore leak into the next row reusing the view, so put them | ||
| * back to the constructor baseline before the binder runs. | ||
| */ | ||
| private fun resetRowStyle() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: [Android style alignment leaks across recycled rows] A styled row with Reset the baseline horizontal gravity for every styleable shared label and add a styled-to-unstyled reuse regression test. |
||
| if (!styledViewsDirty) return | ||
| styledViewsDirty = false | ||
| listOf(title, subtitle, tertiary, status, metricSubtitle).forEach { view -> | ||
| view.typeface = NativeListFonts.regular(context) | ||
| view.setLineSpacing(0f, 1f) | ||
| } | ||
| badgeLine.typeface = NativeListFonts.medium(context) | ||
| badgeLine.setLineSpacing(0f, 1f) | ||
| mediaBadge.typeface = NativeListFonts.regular(context) | ||
| mediaBadge.setLineSpacing(0f, 1f) | ||
| trailingViews.forEach { it.setLineSpacing(0f, 1f) } | ||
| dataColumns.forEach { | ||
| it.typeface = NativeListFonts.medium(context) | ||
| it.setLineSpacing(0f, 1f) | ||
| } | ||
| } | ||
|
|
||
| private fun resetViews() { | ||
| resetRowStyle() | ||
| clipChildren = true | ||
| clipToPadding = true | ||
| selectorOriginalFontFeatures.forEach { (view, original) -> view.fontFeatureSettings = original } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package com.margelo.nitro.nativelist | ||
|
|
||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Test | ||
|
|
||
| class NativeListStyleSlotTest { | ||
| private data class Case( | ||
| val type: String, | ||
| val field: String, | ||
| val variant: String = "", | ||
| val expected: String?, | ||
| ) | ||
|
|
||
| @Test | ||
| fun styleKeyResolvesToTheViewThatRendersThatModelField() { | ||
| val cases = listOf( | ||
| // metricCard swaps the two views: the large number is drawn by the title | ||
| // view and the small label by the subtitle view. | ||
| Case(type = "metricCard", field = "value", expected = "title"), | ||
| Case(type = "metricCard", field = "title", expected = "subtitle"), | ||
| Case(type = "metricCard", field = "subtitle", expected = "metricSubtitle"), | ||
| Case(type = "metricCard", field = "trend", expected = "status"), | ||
| // identity keeps its own names. | ||
| Case(type = "identity", field = "title", expected = "title"), | ||
| Case(type = "identity", field = "valueSecondary", expected = "valueSecondary"), | ||
| // One status view carries four different model fields. | ||
| Case(type = "rail", field = "status", expected = "status"), | ||
| Case(type = "activity", field = "status", expected = "status"), | ||
| Case(type = "message", field = "time", expected = "status"), | ||
| // Amounts, indices and values share the two trailing views. | ||
| Case(type = "activity", field = "primaryAmount", expected = "value"), | ||
| Case(type = "activity", field = "secondaryAmount", expected = "valueSecondary"), | ||
| Case(type = "dataRow", field = "index", expected = "value"), | ||
| Case(type = "sectionHeader", field = "value", expected = "value"), | ||
| // Only the warning variant renders a separate title. | ||
| Case(type = "system", field = "message", variant = "warning", expected = "subtitle"), | ||
| Case(type = "system", field = "message", variant = "noMatch", expected = "title"), | ||
| // A field the template does not render is ignored, never remapped onto | ||
| // whichever view happens to be free. | ||
| Case(type = "identity", field = "price", expected = null), | ||
| Case(type = "rail", field = "subtitle", expected = null), | ||
| Case(type = "market", field = "title", expected = null), | ||
| Case(type = "walletGroup", field = "title", expected = null), | ||
| ) | ||
|
|
||
| assertEquals( | ||
| cases.map(Case::expected), | ||
| cases.map { nativeListStyleSlot(it.type, it.variant, it.field) }, | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun noTemplateMapsTwoStyleKeysOntoOneView() { | ||
| // A collision would make one of the two keys silently win. | ||
| val templates = listOf( | ||
| Triple("identity", "", listOf("title", "subtitle", "tertiary", "badge", "value", "valueSecondary")), | ||
| Triple("rail", "", listOf("title", "badge", "status")), | ||
| Triple("activity", "", listOf("title", "description", "status", "primaryAmount", "secondaryAmount")), | ||
| Triple("message", "", listOf("title", "body", "time")), | ||
| Triple("dataRow", "", listOf("columns", "index")), | ||
| Triple("mediaTile", "", listOf("title", "subtitle", "badge")), | ||
| Triple("metricCard", "", listOf("title", "value", "subtitle", "trend")), | ||
| Triple("sectionHeader", "", listOf("title", "subtitle", "value")), | ||
| Triple("action", "", listOf("title", "value")), | ||
| // Only `warning` carries both a title and a message; the other variants | ||
| // have no title field at all, so their message owning the title view is | ||
| // not a collision. | ||
| Triple("system", "warning", listOf("title", "message", "actionText")), | ||
| ) | ||
|
|
||
| templates.forEach { (type, variant, fields) -> | ||
| val slots = fields.mapNotNull { nativeListStyleSlot(type, variant, it) } | ||
| assertEquals( | ||
| "$type maps two style keys onto one view: $slots", | ||
| slots.size, | ||
| slots.toSet().size, | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: [Data-row secondary styles are silently dropped on native]
When a
dataRowprovides the validstyle.columnSecondaryfield, this mapping returnsnull, so Android ignores it; the new iOS mapping has the same omission. Web renders this slot, and table rows also use separate column-label views that thecolumnspath never reaches.Implement
columnSecondaryin the native linear and table data-row renderers (secondary spans/labels respectively) and cover both layouts with a parity test.