Skip to content

Commit

Permalink
improve comments and names of reportsConstants
Browse files Browse the repository at this point in the history
  • Loading branch information
magicznyleszek committed Sep 23, 2024
1 parent 432a2d6 commit 26b352b
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 29 deletions.
16 changes: 8 additions & 8 deletions jsapp/js/components/reports/chartColorsPicker.component.tsx
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -10,17 +10,17 @@ 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});
}

// 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;
}
Expand All @@ -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'
Expand All @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions jsapp/js/components/reports/chartTypePicker.component.tsx
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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}
Expand Down
10 changes: 5 additions & 5 deletions jsapp/js/components/reports/reportViewItem.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -247,15 +247,15 @@ 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];
}

return opts;
}

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)');
Expand Down
2 changes: 1 addition & 1 deletion jsapp/js/components/reports/reports.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {getDataWithResponses} from './reports.utils';

// Types & constants
import {
REPORT_STYLES,
CHART_STYLES,
DEFAULT_MINIMAL_REPORT_STYLE,
type AssetResponseReportStyles,
type CustomReport,
Expand Down
39 changes: 26 additions & 13 deletions jsapp/js/components/reports/reportsConstants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'),
Expand Down Expand Up @@ -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: [
Expand Down Expand Up @@ -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,
};

0 comments on commit 26b352b

Please sign in to comment.