From 26b352b8b780586355a84ac7acced8c4b09144ee Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Mon, 23 Sep 2024 17:42:05 +0200
Subject: [PATCH] improve comments and names of reportsConstants

---
 .../reports/chartColorsPicker.component.tsx   | 16 ++++----
 .../reports/chartTypePicker.component.tsx     |  4 +-
 .../reports/reportViewItem.component.tsx      | 10 ++---
 jsapp/js/components/reports/reports.tsx       |  2 +-
 .../js/components/reports/reportsConstants.ts | 39 ++++++++++++-------
 5 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/jsapp/js/components/reports/chartColorsPicker.component.tsx b/jsapp/js/components/reports/chartColorsPicker.component.tsx
index 315a536414..a111a768b4 100644
--- a/jsapp/js/components/reports/chartColorsPicker.component.tsx
+++ b/jsapp/js/components/reports/chartColorsPicker.component.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import bem from 'js/bem';
-import {REPORT_COLOR_SETS} from './reportsConstants';
-import type {ReportStyle, ReportColorSet} from './reportsConstants';
+import {CHART_COLOR_SETS} from './reportsConstants';
+import type {ReportStyle, ChartColorSet} from './reportsConstants';
 
 interface ChartColorsPickerProps {
   onChange: (params: {default: boolean}, value: {report_colors: string[]}) => void;
@@ -10,9 +10,9 @@ interface ChartColorsPickerProps {
 
 export default function ChartColorsPicker(props: ChartColorsPickerProps) {
   function defaultReportColorsChange(value: number) {
-    let newColors = REPORT_COLOR_SETS[0].colors;
-    if (REPORT_COLOR_SETS[value]?.colors) {
-      newColors = REPORT_COLOR_SETS[value].colors;
+    let newColors = CHART_COLOR_SETS[0].colors;
+    if (CHART_COLOR_SETS[value]?.colors) {
+      newColors = CHART_COLOR_SETS[value].colors;
     }
 
     props.onChange({default: true}, {report_colors: newColors});
@@ -20,7 +20,7 @@ export default function ChartColorsPicker(props: ChartColorsPickerProps) {
 
   // Not sure why this is called "is default", and not simply "is active". This
   // needs some more investigation.
-  function isDefaultValue(set: ReportColorSet, index: number) {
+  function isDefaultValue(set: ChartColorSet, index: number) {
     if (props.defaultStyle.report_colors === undefined && index === 0) {
       return true;
     }
@@ -33,7 +33,7 @@ export default function ChartColorsPicker(props: ChartColorsPickerProps) {
 
   return (
     <bem.GraphSettings__colors>
-      {REPORT_COLOR_SETS.map((set, index) => (
+      {CHART_COLOR_SETS.map((set, index) => (
         <bem.GraphSettings__radio key={index}>
           <input
             type='radio'
@@ -45,7 +45,7 @@ export default function ChartColorsPicker(props: ChartColorsPickerProps) {
           />
 
           <label htmlFor={'type-' + set.label}>
-            {REPORT_COLOR_SETS[index].colors.map((color, i) => (
+            {CHART_COLOR_SETS[index].colors.map((color, i) => (
               <div style={{backgroundColor: color}} key={i} />
             ))}
           </label>
diff --git a/jsapp/js/components/reports/chartTypePicker.component.tsx b/jsapp/js/components/reports/chartTypePicker.component.tsx
index 76c6eb6da3..17407afa7b 100644
--- a/jsapp/js/components/reports/chartTypePicker.component.tsx
+++ b/jsapp/js/components/reports/chartTypePicker.component.tsx
@@ -1,5 +1,5 @@
 import React from 'react';
-import {REPORT_STYLES} from './reportsConstants';
+import {CHART_STYLES} from './reportsConstants';
 import type {ReportStyle, ReportStyleName} from './reportsConstants';
 import styles from './chartTypePicker.module.scss';
 
@@ -15,7 +15,7 @@ export default function ChartTypePicker(props: ChartTypePickerProps) {
 
   return (
     <section className={styles.root}>
-      {Object.entries(REPORT_STYLES).map(([, styleDefinition], i) => (
+      {Object.entries(CHART_STYLES).map(([, styleDefinition], i) => (
         <div key={i} className={styles.style} data-name={styleDefinition.value}>
           <input
             className={styles.styleInput}
diff --git a/jsapp/js/components/reports/reportViewItem.component.tsx b/jsapp/js/components/reports/reportViewItem.component.tsx
index e670e2ca53..6a0ff188f2 100644
--- a/jsapp/js/components/reports/reportViewItem.component.tsx
+++ b/jsapp/js/components/reports/reportViewItem.component.tsx
@@ -20,7 +20,7 @@ import ReportTable from './reportTable.component';
 import Button from 'js/components/common/button';
 
 // Constants
-import {REPORT_STYLES, REPORT_COLOR_SETS} from './reportsConstants';
+import {CHART_STYLES, CHART_COLOR_SETS} from './reportsConstants';
 
 // Types
 import type {ReportsResponse, ReportsResponseData} from './reportsConstants';
@@ -132,11 +132,11 @@ class ReportViewItem extends React.Component<ReportViewItemProps> {
     Chart.defaults.maintainAspectRatio = false;
 
     // If there is some invalid data we default to bar type
-    const chartJsType: ChartType = REPORT_STYLES[chartType]?.chartJsType || 'bar';
+    const chartJsType: ChartType = CHART_STYLES[chartType]?.chartJsType || 'bar';
 
     const datasets: ChartDataset[] = [];
 
-    const isArea = this.props.style.report_type === REPORT_STYLES.area.value;
+    const isArea = this.props.style.report_type === CHART_STYLES.area.value;
 
     if (data.values !== undefined) {
       if (data.responseLabels) {
@@ -247,7 +247,7 @@ class ReportViewItem extends React.Component<ReportViewItemProps> {
       }
     }
 
-    if (this.props.style.report_type === REPORT_STYLES.area.value) {
+    if (this.props.style.report_type === CHART_STYLES.area.value) {
       opts.data.datasets[0].backgroundColor = colors[0];
     }
 
@@ -255,7 +255,7 @@ class ReportViewItem extends React.Component<ReportViewItemProps> {
   }
 
   buildChartColors() {
-    let output = this.props.style.report_colors || REPORT_COLOR_SETS[0].colors;
+    let output = this.props.style.report_colors || CHART_COLOR_SETS[0].colors;
 
     const c1 = output.slice(0).map((c) => {
       c = c.replace('1)', '0.75)');
diff --git a/jsapp/js/components/reports/reports.tsx b/jsapp/js/components/reports/reports.tsx
index 9904f3e115..5b9986b0b0 100644
--- a/jsapp/js/components/reports/reports.tsx
+++ b/jsapp/js/components/reports/reports.tsx
@@ -27,7 +27,7 @@ import {getDataWithResponses} from './reports.utils';
 
 // Types & constants
 import {
-  REPORT_STYLES,
+  CHART_STYLES,
   DEFAULT_MINIMAL_REPORT_STYLE,
   type AssetResponseReportStyles,
   type CustomReport,
diff --git a/jsapp/js/components/reports/reportsConstants.ts b/jsapp/js/components/reports/reportsConstants.ts
index b0747ca244..513032c2c2 100644
--- a/jsapp/js/components/reports/reportsConstants.ts
+++ b/jsapp/js/components/reports/reportsConstants.ts
@@ -120,6 +120,9 @@ export interface AssetResponseReportCustom {
   [crid: string]: CustomReport;
 }
 
+/**
+ * A name of the report type as KoboToolbox understands it.
+ */
 export type ReportStyleName =
   | 'vertical'
   | 'horizontal'
@@ -132,22 +135,32 @@ export type ReportStyleName =
   | 'polar'
   | 'radar';
 
-interface ReportStyleDefinition {
+/**
+ * Combines together `ReportStyleName`, label (for users) and `ChartType` (from
+ * Chart.js library).
+ */
+interface ChartStyleDefinition {
+  /** This is our internal name of a report style/type. */
   value: ReportStyleName;
+  /**
+   * This is a user friendly version of `ReportStyleName`, we use it to display
+   * all available options in the settings.
+   */
   label: string;
+  /**
+   * This is the name of a chart that Chart.js understands. For some definitions
+   * it matches our internal `ReportStyleName`. We use this name to render
+   * a nice graph/chart in the UI for our users.
+   */
   chartJsType: ChartType;
 }
 
-type ReportStyleDefinitions = {[P in ReportStyleName]: ReportStyleDefinition};
+type ChartStyleDefinitions = {[P in ReportStyleName]: ChartStyleDefinition};
 
 /**
- * A list of definitions of styles. Despite `REPORT_STYLES` (the `const`) name
- * being similar to `ReportStyle` (the `interface`), these are not the same
- * thing. The first is list of definitions of possible styles (mainly for
- * the Chart.js), the latter is the instance of a report style (as Back end
- * understands it).
+ * A list of definitions of chart styles.
  */
-export const REPORT_STYLES: ReportStyleDefinitions = Object.freeze({
+export const CHART_STYLES: ChartStyleDefinitions = Object.freeze({
   vertical: {
     value: 'vertical',
     label: t('Vertical'),
@@ -207,12 +220,12 @@ export const REPORT_STYLES: ReportStyleDefinitions = Object.freeze({
   },
 });
 
-export interface ReportColorSet {
+export interface ChartColorSet {
   label: string;
   colors: string[];
 }
 
-export const REPORT_COLOR_SETS: ReportColorSet[] = [
+export const CHART_COLOR_SETS: ChartColorSet[] = [
   {
     label: 'set1',
     colors: [
@@ -282,9 +295,9 @@ export const REPORT_COLOR_SETS: ReportColorSet[] = [
 
 /**
  * The default report style. An minimal instance of `ReportStyle` that uses
- * values from of `REPORT_STYLES` and `REPORT_COLOR_SETS`.
+ * values from of `CHART_STYLES` and `REPORT_COLOR_SETS`.
  */
 export const DEFAULT_MINIMAL_REPORT_STYLE: ReportStyle = {
-  report_type: REPORT_STYLES.vertical.value,
-  report_colors: REPORT_COLOR_SETS[0].colors,
+  report_type: CHART_STYLES.vertical.value,
+  report_colors: CHART_COLOR_SETS[0].colors,
 };