From dcc29541488505c378b265f1012fb0aa56597f45 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Thu, 25 Jan 2024 00:50:50 +0100 Subject: [PATCH] fix: Show correct trending tag values at the end of the chart lines (#380) The previous code incorrectly showed the trending tag usage data twice next to the end of the trending tag lines, instead of one entry for the usage data and one entry for the account data. Fix that. As part of this fix change how the data is displayed. Instead of using two distinct `TextView`, fixed to the bottom end of the chart, draw the text directly on the chart. The text is accurately position so that it is next to the end of the relevant line. If both lines overlap the label positions are adjusted appropriately. The chart now uses Pachli blue and orange for the line colours. While doing this I discovered that the mechanism used to fall back to particular chart colours if none were specified was incorrect, so fix that too. --- app/lint-baseline.xml | 152 +----------- .../trending/TrendingTagViewHolder.kt | 3 - .../main/java/app/pachli/view/GraphView.kt | 231 ++++++++++-------- .../res/layout-land/item_trending_cell.xml | 47 +--- .../main/res/layout/item_trending_cell.xml | 46 +--- app/src/main/res/values/attrs.xml | 9 - app/src/main/res/values/attrs_graphview.xml | 31 +++ app/src/main/res/values/colors.xml | 8 +- app/src/main/res/values/styles.xml | 8 + 9 files changed, 198 insertions(+), 337 deletions(-) create mode 100644 app/src/main/res/values/attrs_graphview.xml diff --git a/app/lint-baseline.xml b/app/lint-baseline.xml index 8c79fbe87..39a1bd244 100644 --- a/app/lint-baseline.xml +++ b/app/lint-baseline.xml @@ -1433,7 +1433,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~"> @@ -1444,7 +1444,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~"> @@ -2027,7 +2027,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> @@ -2038,7 +2038,7 @@ errorLine2=" ~~~~~~~~~~~~~~~~~~~~~"> @@ -4788,50 +4788,6 @@ column="2"/> - - - - - - - - - - - - - - - - @@ -4850,7 +4806,7 @@ errorLine2=" ~~~~~~~~"> @@ -4861,7 +4817,7 @@ errorLine2=" ~~~~~~~~"> @@ -4872,7 +4828,7 @@ errorLine2=" ~~~~~~~~"> @@ -4883,7 +4839,7 @@ errorLine2=" ~~~~~~~~"> @@ -4894,7 +4850,7 @@ errorLine2=" ~~~~~~~~"> @@ -4986,50 +4942,6 @@ column="9"/> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - = if (isInEditMode) { - listOf( - 30, - 60, - 70, - 80, - 130, - 190, - 80, - ) + listOf(30, 60, 70, 80, 130, 190, 80) } else { - listOf( - 1, - 1, - 1, - 1, - 1, - 1, - 1, - ) + listOf(1, 1, 1, 1, 1, 1, 1) } set(value) { field = value.map { max(1, it) } @@ -96,25 +88,9 @@ class GraphView @JvmOverloads constructor( } var secondaryLineData: List = if (isInEditMode) { - listOf( - 10, - 20, - 40, - 60, - 100, - 132, - 20, - ) + listOf(10, 20, 40, 60, 100, 132, 20) } else { - listOf( - 1, - 1, - 1, - 1, - 1, - 1, - 1, - ) + listOf(1, 1, 1, 1, 1, 1, 1) } set(value) { field = value.map { max(1, it) } @@ -123,48 +99,41 @@ class GraphView @JvmOverloads constructor( } init { - initFromXML(attrs) - } - - private fun initFromXML(attr: AttributeSet?) { - context.obtainStyledAttributes(attr, R.styleable.GraphView).use { a -> - primaryLineColor = context.getColor( - a.getResourceId( - R.styleable.GraphView_primaryLineColor, - R.color.tusky_blue, - ), + context.withStyledAttributes(attrs, R.styleable.GraphView, defStyleAttr, R.style.Pachli_Widget_GraphView) { + primaryLineColor = getColor( + R.styleable.GraphView_primaryLineColor, + MaterialColors.getColor(this@GraphView, com.google.android.material.R.attr.colorPrimary), ) - secondaryLineColor = context.getColor( - a.getResourceId( - R.styleable.GraphView_secondaryLineColor, - R.color.tusky_red, - ), + secondaryLineColor = getColor( + R.styleable.GraphView_secondaryLineColor, + MaterialColors.getColor(this@GraphView, com.google.android.material.R.attr.colorSecondary), ) - lineWidth = a.getDimensionPixelSize( + metaColor = getColor( + R.styleable.GraphView_metaColor, + MaterialColors.getColor(this@GraphView, com.google.android.material.R.attr.colorOutline), + ) + + lineWidth = getDimensionPixelSize( R.styleable.GraphView_lineWidth, R.dimen.graph_line_thickness, ).toFloat() - graphColor = context.getColor( - a.getResourceId( - R.styleable.GraphView_graphColor, - android.R.attr.colorBackground, - ), + graphColor = getColor( + R.styleable.GraphView_graphColor, + MaterialColors.getColor(this@GraphView, android.R.attr.colorBackground), ) - metaColor = context.getColor( - a.getResourceId( - R.styleable.GraphView_metaColor, - com.google.android.material.R.attr.dividerColor, - ), - ) - - proportionalTrending = a.getBoolean( + proportionalTrending = getBoolean( R.styleable.GraphView_proportionalTrending, proportionalTrending, ) + + labelTextSize = getDimensionPixelSize( + R.styleable.GraphView_labelTextSize, + labelTextSize.toInt(), + ).toFloat() } primaryLinePaint = Paint(Paint.ANTI_ALIAS_FLAG).apply { @@ -189,6 +158,20 @@ class GraphView @JvmOverloads constructor( style = Paint.Style.FILL } + primaryTextPaint = Paint(Paint.ANTI_ALIAS_FLAG).apply { + color = primaryLineColor + style = Paint.Style.FILL + textSize = labelTextSize + textAlign = Paint.Align.RIGHT + } + + secondaryTextPaint = Paint(Paint.ANTI_ALIAS_FLAG).apply { + color = secondaryLineColor + style = Paint.Style.FILL + textSize = labelTextSize + textAlign = Paint.Align.RIGHT + } + graphPaint = Paint(Paint.ANTI_ALIAS_FLAG).apply { color = graphColor } @@ -198,6 +181,14 @@ class GraphView @JvmOverloads constructor( strokeWidth = 0f style = Paint.Style.STROKE } + + // Determine how much padding to leave on the right/end of the chart so there's + // space for the labels. The widest possible label string is "1000.0M", so + // compute that width, with some additional space on the left to separate the + // label from the line. + val labelBounds = Rect() + primaryTextPaint.getTextBounds("1000.0M", 0, 7, labelBounds) + paddingEnd = (4 * lineWidth) + labelBounds.width() + labelBounds.left } private fun initializeVertices() { @@ -264,7 +255,7 @@ class GraphView @JvmOverloads constructor( } } - private fun dataSpacing(data: List) = width.toFloat() / max(data.size - 1, 1).toFloat() + private fun dataSpacing(data: List) = (width.toFloat() - paddingEnd) / max(data.size - 1, 1).toFloat() override fun onDraw(canvas: Canvas) { super.onDraw(canvas) @@ -279,7 +270,7 @@ class GraphView @JvmOverloads constructor( val pointDistance = dataSpacing(primaryLineData) // Vertical tick marks - for (i in 0 until primaryLineData.size + 1) { + for (i in primaryLineData.indices) { drawLine( i * pointDistance, height.toFloat(), @@ -290,7 +281,7 @@ class GraphView @JvmOverloads constructor( } // X-axis - drawLine(0f, height.toFloat(), width.toFloat(), height.toFloat(), metaPaint) + drawLine(0f, height.toFloat(), width.toFloat() - paddingEnd, height.toFloat(), metaPaint) // Data lines drawLine( @@ -298,36 +289,84 @@ class GraphView @JvmOverloads constructor( linePath = secondaryLinePath, linePaint = secondaryLinePaint, circlePaint = secondaryCirclePaint, - lineThickness = lineWidth, ) drawLine( canvas = canvas, linePath = primaryLinePath, linePaint = primaryLinePaint, circlePaint = primaryCirclePaint, - lineThickness = lineWidth, + ) + + // Data text + drawEndText( + canvas, + formatNumber(primaryLineData.last(), 1000), + formatNumber(secondaryLineData.last(), 1000), + primaryLinePath, + secondaryLinePath, ) } } - private fun drawLine( - canvas: Canvas, - linePath: Path, - linePaint: Paint, - circlePaint: Paint, - lineThickness: Float, - ) { + private fun drawLine(canvas: Canvas, linePath: Path, linePaint: Paint, circlePaint: Paint) { canvas.apply { - drawPath( - linePath, - linePaint, - ) - - val pm = PathMeasure(linePath, false) - val coord = floatArrayOf(0f, 0f) - pm.getPosTan(pm.length * 1f, coord, null) - - drawCircle(coord[0], coord[1], lineThickness * 2f, circlePaint) + drawPath(linePath, linePaint) + val (x, y) = pathEnd(linePath) + drawCircle(x, y, lineWidth * 2f, circlePaint) } } + + private fun drawEndText( + canvas: Canvas, + primaryValue: String, + secondaryValue: String, + primaryLinePath: Path, + secondaryLinePath: Path, + ) { + var (primaryX, primaryY) = pathEnd(primaryLinePath) + var (_, secondaryY) = pathEnd(secondaryLinePath) + + val primaryBounds = Rect() + val secondaryBounds = Rect() + primaryTextPaint.getTextBounds(primaryValue, 0, primaryValue.length, primaryBounds) + secondaryTextPaint.getTextBounds(secondaryValue, 0, secondaryValue.length, secondaryBounds) + + // Adjust both texts to horizontally align with their respective circle endpoints + primaryY += primaryBounds.height().toFloat() / 2 + secondaryY += secondaryBounds.height().toFloat() / 2 + + // Force the two apart if they overlap + val overlap = primaryY - (secondaryY - secondaryBounds.height()) + // First try and force them both apart + if (overlap > 0) { + secondaryY += (overlap / 2) + 5 + primaryY -= (overlap / 2) + 5 + } + // Now, if secondary is off the canvas move both of them up to compensate + val secondaryClip = secondaryY - canvas.height + if (secondaryClip > 0) { + secondaryY -= secondaryClip + primaryY -= secondaryClip + } + + // The number text is right aligned to ensure they line up. The primary text + // (total usage) is always going to be larger than the secondary text, so use + // that to determine the X position of the right-hand edge of the text. This is: + // - primaryX + // - + 4 * lineWidth (spacing between the line circle and the text) + // - + primaryBounds.width() (width of the text) + // - + primaryBounds.left (left margin of the text) + val textX = primaryX + (4 * lineWidth) + primaryBounds.width() + primaryBounds.left + canvas.apply { + drawText(primaryValue, textX, primaryY, primaryTextPaint) + drawText(secondaryValue, textX, secondaryY, secondaryTextPaint) + } + } + + private fun pathEnd(path: Path): Pair { + val pm = PathMeasure(path, false) + val coord = floatArrayOf(0f, 0f) + pm.getPosTan(pm.length * 1f, coord, null) + return Pair(coord[0], coord[1]) + } } diff --git a/app/src/main/res/layout-land/item_trending_cell.xml b/app/src/main/res/layout-land/item_trending_cell.xml index 541dfd256..1b380e957 100644 --- a/app/src/main/res/layout-land/item_trending_cell.xml +++ b/app/src/main/res/layout-land/item_trending_cell.xml @@ -21,48 +21,10 @@ android:importantForAccessibility="no" app:graphColor="?android:colorBackground" app:layout_constraintBottom_toBottomOf="parent" - app:layout_constraintEnd_toStartOf="@id/current_usage" + app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" - app:lineWidth="2sp" - app:metaColor="@color/tusky_blue_light" - app:primaryLineColor="@color/tusky_blue" - app:proportionalTrending="true" - app:secondaryLineColor="@color/tusky_red" /> - - - - + app:proportionalTrending="true" /> - diff --git a/app/src/main/res/layout/item_trending_cell.xml b/app/src/main/res/layout/item_trending_cell.xml index 97e48bd60..2c48cdba7 100644 --- a/app/src/main/res/layout/item_trending_cell.xml +++ b/app/src/main/res/layout/item_trending_cell.xml @@ -21,48 +21,10 @@ android:importantForAccessibility="no" app:graphColor="?android:colorBackground" app:layout_constraintBottom_toBottomOf="parent" - app:layout_constraintEnd_toStartOf="@id/current_usage" + app:layout_constraintEnd_toEndOf="parent" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" - app:lineWidth="2sp" - app:metaColor="@color/tusky_blue_light" - app:primaryLineColor="@color/tusky_blue" - app:proportionalTrending="true" - app:secondaryLineColor="@color/tusky_red" /> - - - - + app:proportionalTrending="true" /> - - - - - - - - - diff --git a/app/src/main/res/values/attrs_graphview.xml b/app/src/main/res/values/attrs_graphview.xml new file mode 100644 index 000000000..9b9082135 --- /dev/null +++ b/app/src/main/res/values/attrs_graphview.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml index 0f0579888..3731f7020 100644 --- a/app/src/main/res/values/colors.xml +++ b/app/src/main/res/values/colors.xml @@ -65,12 +65,16 @@ #2b90d9 - #56a7e1 #ca8f04 #fab207 #19a341 #25d069 - #DF1553 + + #006382 + #F37B2F + + + #fff #000 diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index 9d5628fa0..5b7b48717 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -80,6 +80,8 @@ @android:color/transparent ?attr/isLightTheme + + @style/Pachli.Widget.GraphView + +