Identify parents of comment by using colored lines #49

mvietri · 2019-08-18T03:55:30Z

Toggle added to Look & Feel Preferences.

tunous-bot · 2019-08-18T09:51:59Z

Test version of Dank has been automatically built from this pull request.
Click here to download it.

Tunous

Looks nice so far. I've left some feedback with suggestions with what to change.

I don't think that there is a need for allowing users to change the colors to custom ones. We just have to find some nice defaults that will work with the current style of the app. @dotvhs I'll mention you here as you might be able to find good colors for that :)

app/src/main/java/me/saket/dank/widgets/IndentedLayout.java

tunous-bot · 2019-08-18T12:35:15Z

Test version of Dank has been automatically built from this pull request.
Click here to download it.

…iber and change color pallete

tunous-bot · 2019-08-18T20:06:49Z

Test version of Dank has been automatically built from this pull request.
Click here to download it.

Tunous

Almost there 👍

app/src/main/java/me/saket/dank/widgets/IndentedLayout.java

tunous-bot · 2019-08-19T04:43:04Z

Test version of Dank has been automatically built from this pull request.
Click here to download it.

Identify parents of comment by using colored lines * github.com:Tunous/Dank: improved legibility and fix performance issues Move arraylist initialization to a void method, add preference subscriber and change color pallete Inject IntededLayout to use user preferences Read value from preferences and fixed Add depth color to comment's children and toggle in preferences

Tunous

Final run through the code, this time I've mostly checked and listed what Android Studio suggests :)

app/src/main/java/me/saket/dank/widgets/IndentedLayout.java

Tunous · 2019-08-20T19:04:26Z

I also noticed that the preference change did not work correctly. The tree was being reset on preference change but since the lines were being actually created in onLayout our change from observer only destroyed old lines. I'm attaching a patch which fixes this by only updating colors on preference change instead of recreating whole tree. It contains fixes from my previous comments too. I'm not commiting this myself because I have no idea what colors we should actually use. I'll leave that to you :)

patch

diff --git a/app/src/main/res/values/colors.xml b/app/src/main/res/values/colors.xml
index ae3de16cfc8d429b134987d02fb1ee7375b2946a..9f7426d8407f679779e59b772d752ebe418dc2ef 100644
--- a/app/src/main/res/values/colors.xml
+++ b/app/src/main/res/values/colors.xml
@@ -94,4 +94,18 @@
   <!-- Text -->
   <color name="text_primary">@color/gray_200</color>
   <color name="text_secondary">@color/gray_500</color>
+
+  <array name="indentation_colors">
+    <item>@color/teal_200</item>
+    <item>@color/teal_300</item>
+    <item>@color/teal_400</item>
+    <item>@color/teal_500</item>
+    <item>@color/teal_600</item>
+    <item>@color/teal_700</item>
+    <item>@color/teal_800</item>
+    <item>@color/teal_A200</item>
+    <item>@color/teal_A400</item>
+    <item>@color/teal_A500</item>
+    <item>@color/teal_A700</item>
+  </array>
 </resources>
diff --git a/app/src/main/java/me/saket/dank/widgets/IndentedLayout.java b/app/src/main/java/me/saket/dank/widgets/IndentedLayout.java
index cd6c171358c196a7fb75ef6c75ed0b2d2476aa31..9a6969af2e7f9a2a9d4d7d7c50c62d145d960450 100644
--- a/app/src/main/java/me/saket/dank/widgets/IndentedLayout.java
+++ b/app/src/main/java/me/saket/dank/widgets/IndentedLayout.java
@@ -7,19 +7,17 @@ import android.graphics.Color;
 import android.graphics.DashPathEffect;
 import android.graphics.Paint;
 import android.graphics.Path;
-import androidx.annotation.IntRange;
+import android.util.AttributeSet;
+import android.util.TypedValue;
+import android.widget.LinearLayout;
+
 import androidx.annotation.Nullable;
 import androidx.annotation.Px;
 import com.f2prateek.rx.preferences2.Preference;
 import com.jakewharton.rxbinding2.view.RxView;
 
-import android.util.AttributeSet;
-import android.util.TypedValue;
-import android.widget.LinearLayout;
-
 import java.util.ArrayList;
 import java.util.List;
-
 import javax.inject.Inject;
 import javax.inject.Named;
 
@@ -30,36 +28,15 @@ public class IndentedLayout extends LinearLayout {
   private static final int DEFAULT_SPACING_PER_DEPTH_DP = 10;
   private static final int DEFAULT_LINE_WIDTH = 6;
 
-  private int indentationDepth;
   private final int spacePerDepthPx;
-  private Paint indentationLinePaint;
-  private int indentationLineWidth;
-  private int defaultIndentationLineColor;
-
-  private int originalPaddingStart;
-
-  private static final String[] INDENTATION_COLORS = {
-      "#FFC40D", // Yellow
-      "#2D89EF", // Blue
-      "#B91D47", // Dark Red
-      "#00ABA9", // Teal
-      "#E3A21A", // Orange
-      "#99B433", // Light Green
-      "#7E3878", // Purple
-      "#FFB300", // Vivid Yellow
-      "#FFFFFF", // White
-      "#00A300", // Green
-      "#2B5797", // Dark Blue
-      "#9F00A7", // Light Purple
-      "#603CBA", // Dark Purple
-      "#EE1111", // Red
-      "#EFF4FF", // Light Blue
-      "#DA532C", // Dark Orange
-      "#FF0097", // Magenta
-      "#1E7145", // Dark Green
-  };
+  private final Paint indentationLinePaint;
+  private final int defaultIndentationLineColor;
+  private final int originalPaddingStart;
+  private final int[] indentationColors;
 
+  private int indentationDepth;
   private List<ColoredTree> trees = new ArrayList<>();
+
   @Inject @Named("show_colored_comments_tree") Preference<Boolean> coloredDepthPreference;
 
   public IndentedLayout(Context context, @Nullable AttributeSet attrs) {
@@ -68,19 +45,12 @@ public class IndentedLayout extends LinearLayout {
 
     Dank.dependencyInjector().inject(this);
 
-    coloredDepthPreference.asObservable()
-        .takeUntil(RxView.detaches(this))
-        .subscribe(o -> {
-          setupDepthLines();
-          invalidate();
-        });
-
-    indentationLinePaint = new Paint();
     originalPaddingStart = getPaddingStart();
+    indentationColors = getResources().getIntArray(R.array.indentation_colors);
 
     TypedArray attributes = context.obtainStyledAttributes(attrs, R.styleable.IndentedLayout);
     spacePerDepthPx = attributes.getDimensionPixelSize(R.styleable.IndentedLayout_spacePerDepth, dpToPx(DEFAULT_SPACING_PER_DEPTH_DP, context));
-    indentationLineWidth = attributes.getDimensionPixelSize(R.styleable.IndentedLayout_indentationLineWidth, dpToPx(DEFAULT_LINE_WIDTH, context));
+    int indentationLineWidth = attributes.getDimensionPixelSize(R.styleable.IndentedLayout_indentationLineWidth, dpToPx(DEFAULT_LINE_WIDTH, context));
     defaultIndentationLineColor = attributes.getColor(R.styleable.IndentedLayout_indentationLineColor, Color.LTGRAY);
     attributes.recycle();
 
@@ -89,6 +59,13 @@ public class IndentedLayout extends LinearLayout {
     indentationLinePaint.setStrokeWidth(indentationLineWidth);
     indentationLinePaint.setStyle(Paint.Style.FILL_AND_STROKE);
     indentationLinePaint.setPathEffect(new DashPathEffect(new float[] { indentationLineWidth * 2, indentationLineWidth * 2 }, 0));
+
+    coloredDepthPreference.asObservable()
+        .takeUntil(RxView.detaches(this))
+        .subscribe(colored -> {
+          updateLineColors(colored);
+          invalidate();
+        });
   }
 
   @Override
@@ -116,10 +93,10 @@ public class IndentedLayout extends LinearLayout {
     }
   }
 
-  public void setIndentationDepth(@IntRange(from = 0, to = 1) int depth) {
+  public void setIndentationDepth(int depth) {
     indentationDepth = depth;
 
-    setupDepthLines();
+    setupDepthLines(coloredDepthPreference.get());
 
     int indentationSpacing = (int) (indentationDepth * spacePerDepthPx + indentationLinePaint.getStrokeWidth());
     setPaddingRelative(originalPaddingStart + indentationSpacing, getPaddingTop(), getPaddingEnd(), getPaddingBottom());
@@ -127,24 +104,33 @@ public class IndentedLayout extends LinearLayout {
     invalidate();
   }
 
-  private void setupDepthLines() {
+  private void setupDepthLines(Boolean colored) {
     trees = new ArrayList<>();
 
     for (int i = 0; i < indentationDepth; i++) {
-      int colorIndex = i % INDENTATION_COLORS.length;
-      int depthColor = coloredDepthPreference.get() ? Color.parseColor(INDENTATION_COLORS[colorIndex]) : defaultIndentationLineColor;
-
+      int depthColor = getDepthColor(i, colored);
       trees.add(new ColoredTree(depthColor, new Path()));
     }
   }
 
+  private int getDepthColor(int index, Boolean shouldBeColored) {
+    int colorIndex = index % indentationColors.length;
+    return shouldBeColored ? indentationColors[colorIndex] : defaultIndentationLineColor;
+  }
+
+  private void updateLineColors(Boolean colored) {
+    for (int i = 0; i < trees.size(); i++) {
+      trees.get(i).color = getDepthColor(i, colored);
+    }
+  }
+
   @Px
   public static int dpToPx(float dpValue, Context context) {
     return (int) TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, dpValue, context.getResources().getDisplayMetrics());
   }
 
   private static class ColoredTree {
-    public final int color;
+    public int color;
     public final Path path;
 
     public ColoredTree(int color, Path path) {

Details

Tunous · 2019-08-20T19:10:07Z

One final optimization is to add skip(1) before takeUntil to the subscription to remove unnecessary view invalidation when the preference is first read as we are only interested in subsequent changes.

mvietri · 2019-08-21T12:38:37Z

Thanks @Tunous . I'll be patching this when I can. Should I go and convert to Kotlin?

Tunous · 2019-08-21T12:42:51Z

I don't think there is need for that here.

mvietri · 2019-08-22T00:13:41Z

All done

tunous-bot · 2019-08-22T04:33:35Z

Test version of Dank has been automatically built from this pull request.
Click here to download it.

tunous-bot · 2019-08-22T15:35:12Z

Test version of Dank has been automatically built from this pull request.
Click here to download it.

Add depth color to comment's children and toggle in preferences

bd2f04c

Tunous suggested changes Aug 18, 2019

View reviewed changes

Tunous added the feature New feature label Aug 18, 2019

Mauro Vietri added 2 commits August 18, 2019 09:03

Read value from preferences and fixed

4e2e948

Inject IntededLayout to use user preferences

2ae7217

Move arraylist initialization to a void method, add preference subscr…

276484e

…iber and change color pallete

Tunous suggested changes Aug 18, 2019

View reviewed changes

improved legibility and fix performance issues

f0c74e3

mvietri marked this pull request as ready for review August 19, 2019 21:25

Tunous suggested changes Aug 20, 2019

View reviewed changes

PR Comments applied

177241d

Tunous approved these changes Aug 22, 2019

View reviewed changes

Tunous added 2 commits August 22, 2019 17:19

Add changelog entry

3741c89

Merge branch 'master' into feature/colored_comment_tree

b55aa29

Tunous added this to the 0.8.0 milestone Aug 22, 2019

Tunous merged commit 4b21a5f into Tunous:master Aug 22, 2019

mvietri deleted the feature/colored_comment_tree branch August 22, 2019 16:51

mvietri pushed a commit to mvietri/Dawn that referenced this pull request Aug 24, 2019

Identify parents of comment by using colored lines (Tunous#49)

5fbc386

Identify parents of comment by using colored lines #49

Identify parents of comment by using colored lines #49

Uh oh!

Conversation

mvietri commented Aug 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tunous-bot commented Aug 18, 2019

Uh oh!

Tunous left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tunous-bot commented Aug 18, 2019

Uh oh!

tunous-bot commented Aug 18, 2019

Uh oh!

Tunous left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tunous-bot commented Aug 19, 2019

Uh oh!

Tunous left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Tunous commented Aug 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tunous commented Aug 20, 2019

Uh oh!

mvietri commented Aug 21, 2019

Uh oh!

Tunous commented Aug 21, 2019

Uh oh!

mvietri commented Aug 22, 2019

Uh oh!

tunous-bot commented Aug 22, 2019

Uh oh!

tunous-bot commented Aug 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mvietri commented Aug 18, 2019 •

edited

Loading

Tunous commented Aug 20, 2019 •

edited

Loading