Skip to content
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

Convert RecurringSchedulePicker.jsx -> RecurringSchedulePicker.tsx #3396

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/desktop-client/src/components/common/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ function isValueOption<Value>(
export type SelectOption<Value = string> = [Value, string] | typeof Menu.line;

type SelectProps<Value> = {
id?: string;
bare?: boolean;
options: Array<readonly [Value, string] | typeof Menu.line>;
value: Value;
Expand All @@ -41,6 +42,7 @@ type SelectProps<Value> = {
* // <Select options={[['1', 'Option 1'], ['2', 'Option 2']]} value="3" defaultLabel="Select an option" onChange={handleOnChange} />
*/
export function Select<const Value = string>({
id,
bare,
options,
value,
Expand All @@ -61,6 +63,7 @@ export function Select<const Value = string>({
<>
<Button
ref={triggerRef}
id={id}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ question: ‏why do we need an id on the buttons? And what does it do?

Copy link
Contributor Author

@jfdoming jfdoming Sep 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used for the htmlFor attributes that were pre-existing in RecurringSchedulePicker. The id attribute was actually being passed as a prop but the labels weren't being correctly associated with the fields since Select didn't actually accept that prop. I suppose you can consider this an additional bugfix caught by static typing since they were supposed to be associated?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah.. ok, that makes sense for input fields. But not so much for buttons I think.

Copy link
Contributor Author

@jfdoming jfdoming Sep 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which element should we put it on here? The htmlFor is supposed to be for an input element, but the "input" in this case is the Select.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 ignore my previous comment. I'm reviewing too many things in parallel.

--

It makes sense to put it on the Button here since the general element is Select and the button is used for opening the select popover.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no worries!

type={bare ? 'bare' : 'normal'}
disabled={disabled}
onClick={() => {
Expand Down
6 changes: 2 additions & 4 deletions packages/desktop-client/src/components/forms.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { type ReactNode, type HTMLProps } from 'react';
import React, { type ReactNode, type ComponentProps } from 'react';

import { css } from 'glamor';

Expand Down Expand Up @@ -64,9 +64,7 @@ export const FormField = ({ style, children }: FormFieldProps) => {

// Custom inputs

type CheckboxProps = Omit<HTMLProps<HTMLInputElement>, 'type'> & {
style?: CSSProperties;
};
type CheckboxProps = ComponentProps<'input'>;

export const Checkbox = (props: CheckboxProps) => {
return (
Expand Down
3 changes: 3 additions & 0 deletions packages/desktop-client/src/components/select/DateSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ function defaultShouldSaveFromKey(e) {
}

type DateSelectProps = {
id?: string;
containerProps?: ComponentProps<typeof View>;
inputProps?: ComponentProps<typeof Input>;
value: string;
Expand All @@ -188,6 +189,7 @@ type DateSelectProps = {
};

export function DateSelect({
id,
containerProps,
inputProps,
value: defaultValue,
Expand Down Expand Up @@ -344,6 +346,7 @@ export function DateSelect({
return (
<View {...containerProps}>
<Input
id={id}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same Q here

focused={focused}
{...inputProps}
inputRef={inputRef}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
import React, { useEffect, useReducer, useRef, useState } from 'react';
import {
type CSSProperties,
type Dispatch,
useEffect,
useReducer,
useRef,
useState,
} from 'react';

import { sendCatch } from 'loot-core/src/platform/client/fetch';
import * as monthUtils from 'loot-core/src/shared/months';
import { getRecurringDescription } from 'loot-core/src/shared/schedules';
import { type RecurConfig, type RecurPattern } from 'loot-core/types/models';
import { type WithRequired } from 'loot-core/types/util';

import { useDateFormat } from '../../hooks/useDateFormat';
import { SvgAdd, SvgSubtract } from '../../icons/v0';
Expand All @@ -28,7 +37,7 @@ const FREQUENCY_OPTIONS = [
{ id: 'weekly', name: 'Weeks' },
{ id: 'monthly', name: 'Months' },
{ id: 'yearly', name: 'Years' },
];
] as const;

const DAY_OF_MONTH_OPTIONS = [...Array(31).keys()].map(day => day + 1);

Expand All @@ -40,16 +49,16 @@ const DAY_OF_WEEK_OPTIONS = [
{ id: 'TH', name: 'Thursday' },
{ id: 'FR', name: 'Friday' },
{ id: 'SA', name: 'Saturday' },
];
] as const;

function parsePatternValue(value) {
function parsePatternValue(value: string | number) {
if (value === 'last') {
return -1;
}
return Number(value);
}

function parseConfig(config) {
function parseConfig(config: Partial<RecurConfig>): StateConfig {
return {
start: monthUtils.currentDay(),
interval: 1,
Expand All @@ -58,28 +67,37 @@ function parseConfig(config) {
skipWeekend: false,
weekendSolveMode: 'before',
endMode: 'never',
endOccurrences: '1',
endOccurrences: 1,
endDate: monthUtils.currentDay(),
...config,
};
}

function unparseConfig(parsed) {
function unparseConfig(parsed: StateConfig): RecurConfig {
return {
...parsed,
interval: validInterval(parsed.interval),
endOccurrences: validInterval(parsed.endOccurrences),
};
}

function createMonthlyRecurrence(startDate) {
function createMonthlyRecurrence(startDate: string) {
return {
value: parseInt(monthUtils.format(startDate, 'd')),
type: 'day',
type: 'day' as const,
};
}

function boundedRecurrence({ field, value, recurrence }) {
function boundedRecurrence({
field,
value,
recurrence,
}: {
recurrence: RecurPattern;
} & (
| { field: 'type'; value: RecurPattern['type'] }
| { field: 'value'; value: RecurPattern['value'] }
)) {
if (
(field === 'value' &&
recurrence.type !== 'day' &&
Expand All @@ -93,7 +111,48 @@ function boundedRecurrence({ field, value, recurrence }) {
return { [field]: value };
}

function reducer(state, action) {
type StateConfig = Omit<
WithRequired<RecurConfig, 'patterns' | 'endDate' | 'weekendSolveMode'>,
'interval' | 'endOccurrences'
> & {
interval: number | string;
endOccurrences: number | string;
};

type ReducerState = {
config: StateConfig;
};

type UpdateRecurrenceAction =
| {
type: 'update-recurrence';
recurrence: RecurPattern;
field: 'type';
value: RecurPattern['type'];
}
| {
type: 'update-recurrence';
recurrence: RecurPattern;
field: 'value';
value: RecurPattern['value'];
};

type ChangeFieldAction<T extends keyof StateConfig> = {
type: 'change-field';
field: T;
value: StateConfig[T];
};

type ReducerAction =
| { type: 'replace-config'; config: StateConfig }
| ChangeFieldAction<keyof StateConfig>
| UpdateRecurrenceAction
| { type: 'add-recurrence' }
| { type: 'remove-recurrence'; recurrence: RecurPattern }
| { type: 'set-skip-weekend'; skipWeekend: boolean }
| { type: 'set-weekend-solve'; value: StateConfig['weekendSolveMode'] };

function reducer(state: ReducerState, action: ReducerAction): ReducerState {
switch (action.type) {
case 'replace-config':
return { ...state, config: action.config };
Expand Down Expand Up @@ -159,7 +218,7 @@ function reducer(state, action) {
}
}

function SchedulePreview({ previewDates }) {
function SchedulePreview({ previewDates }: { previewDates: Date[] }) {
const dateFormat = (useDateFormat() || 'MM/dd/yyyy')
.replace('MM', 'M')
.replace('dd', 'd');
Expand Down Expand Up @@ -198,15 +257,18 @@ function SchedulePreview({ previewDates }) {
);
}

function validInterval(interval) {
const intInterval = parseInt(interval);
function validInterval(interval: string | number) {
const intInterval = Number(interval);
return Number.isInteger(intInterval) && intInterval > 0 ? intInterval : 1;
}

function MonthlyPatterns({ config, dispatch }) {
const updateRecurrence = (recurrence, field, value) =>
dispatch({ type: 'update-recurrence', recurrence, field, value });

function MonthlyPatterns({
config,
dispatch,
}: {
config: StateConfig;
dispatch: Dispatch<ReducerAction>;
}) {
return (
<Stack spacing={2} style={{ marginTop: 10 }}>
{config.patterns.map((recurrence, idx) => (
Expand All @@ -221,24 +283,34 @@ function MonthlyPatterns({ config, dispatch }) {
options={[
[-1, 'Last'],
Menu.line,
...DAY_OF_MONTH_OPTIONS.map(opt => [opt, opt]),
...DAY_OF_MONTH_OPTIONS.map(opt => [opt, String(opt)] as const),
]}
value={recurrence.value}
onChange={value =>
updateRecurrence(recurrence, 'value', parsePatternValue(value))
dispatch({
type: 'update-recurrence',
recurrence,
field: 'value',
value: parsePatternValue(value),
})
}
disabledKeys={['-']}
buttonStyle={{ flex: 1, marginRight: 10 }}
/>
<Select
options={[
['day', 'Day'],
Menu.line,
...DAY_OF_WEEK_OPTIONS.map(opt => [opt.id, opt.name]),
...DAY_OF_WEEK_OPTIONS.map(opt => [opt.id, opt.name] as const),
]}
value={recurrence.type}
onChange={value => updateRecurrence(recurrence, 'type', value)}
disabledKeys={['-']}
onChange={value => {
dispatch({
type: 'update-recurrence',
recurrence,
field: 'type',
value,
});
}}
buttonStyle={{ flex: 1, marginRight: 10 }}
/>
<Button
Expand Down Expand Up @@ -268,7 +340,15 @@ function MonthlyPatterns({ config, dispatch }) {
);
}

function RecurringScheduleTooltip({ config: currentConfig, onClose, onSave }) {
function RecurringScheduleTooltip({
config: currentConfig,
onClose,
onSave,
}: {
config: RecurConfig;
onClose: () => void;
onSave: (config: RecurConfig) => void;
}) {
const [previewDates, setPreviewDates] = useState(null);

const [state, dispatch] = useReducer(reducer, {
Expand All @@ -289,8 +369,10 @@ function RecurringScheduleTooltip({ config: currentConfig, onClose, onSave }) {

const { config } = state;

const updateField = (field, value) =>
dispatch({ type: 'change-field', field, value });
const updateField = <Field extends keyof RecurConfig>(
field: Field,
value: StateConfig[Field],
) => dispatch({ type: 'change-field', field, value });

useEffect(() => {
async function run() {
Expand Down Expand Up @@ -382,7 +464,6 @@ function RecurringScheduleTooltip({ config: currentConfig, onClose, onSave }) {
<Button
style={{
backgroundColor: theme.tableBackground,
':hover': { backgroundColor: theme.tableBackground },
}}
onPress={() => dispatch({ type: 'add-recurrence' })}
>
Expand Down Expand Up @@ -460,12 +541,22 @@ function RecurringScheduleTooltip({ config: currentConfig, onClose, onSave }) {
);
}

export function RecurringSchedulePicker({ value, buttonStyle, onChange }) {
type RecurringSchedulePickerProps = {
value: RecurConfig;
buttonStyle?: CSSProperties;
onChange: (config: RecurConfig) => void;
};

export function RecurringSchedulePicker({
value,
buttonStyle,
onChange,
}: RecurringSchedulePickerProps) {
const triggerRef = useRef(null);
const [isOpen, setIsOpen] = useState(false);
const dateFormat = useDateFormat() || 'MM/dd/yyyy';

function onSave(config) {
function onSave(config: RecurConfig) {
onChange(config);
setIsOpen(false);
}
Expand Down
32 changes: 18 additions & 14 deletions packages/loot-core/src/types/models/schedule.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,23 @@ import type { AccountEntity } from './account';
import type { PayeeEntity } from './payee';
import type { RuleEntity } from './rule';

export interface RecurPattern {
value: number;
type: 'SU' | 'MO' | 'TU' | 'WE' | 'TH' | 'FR' | 'SA' | 'day';
}

export interface RecurConfig {
frequency: 'daily' | 'weekly' | 'monthly' | 'yearly';
interval: number;
patterns?: RecurPattern[];
skipWeekend?: boolean;
start: string;
endMode: 'never' | 'after_n_occurrences' | 'on_date';
endOccurrences?: number;
endDate?: string;
weekendSolveMode?: 'before' | 'after';
}

export interface ScheduleEntity {
id: string;
name?: string;
Expand All @@ -17,20 +34,7 @@ export interface ScheduleEntity {
_account: AccountEntity['id'];
_amount: unknown;
_amountOp: string;
_date: {
interval: number;
patterns: {
value: number;
type: 'SU' | 'MO' | 'TU' | 'WE' | 'TH' | 'FR' | 'SA' | 'day';
}[];
skipWeekend: boolean;
start: string;
endMode: 'never' | 'after_n_occurrences' | 'on_date';
endOccurrences: number;
endDate: string;
weekendSolveMode: 'before' | 'after';
frequency: 'daily' | 'weekly' | 'monthly' | 'yearly';
};
_date: RecurConfig;
_conditions: unknown;
_actions: unknown;
}
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/3396.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [jfdoming]
---

Convert `RecurringSchedulePicker.jsx` -> `RecurringSchedulePicker.tsx`
Loading