From 3b49d5e4299c4c48dcc973efd1d093f459e62558 Mon Sep 17 00:00:00 2001 From: thefirex Date: Fri, 4 Oct 2024 22:29:51 +0100 Subject: [PATCH 1/4] Fixes focus ring getting stuck on last column of /accounts/budgeted screen when creating a new transaction. --- .../src/components/common/Button2.tsx | 15 +++++++-- .../transactions/TransactionsTable.jsx | 31 +++++++++++++++++-- upcoming-release-notes/3571.md | 6 ++++ 3 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 upcoming-release-notes/3571.md diff --git a/packages/desktop-client/src/components/common/Button2.tsx b/packages/desktop-client/src/components/common/Button2.tsx index 694429d573c..275c6924746 100644 --- a/packages/desktop-client/src/components/common/Button2.tsx +++ b/packages/desktop-client/src/components/common/Button2.tsx @@ -1,6 +1,7 @@ import React, { forwardRef, useMemo, + useRef, type ComponentPropsWithoutRef, type ReactNode, } from 'react'; @@ -15,6 +16,8 @@ import { AnimatedLoading } from '../../icons/AnimatedLoading'; import { type CSSProperties, styles, theme } from '../../style'; import { View } from './View'; +import { useProperFocus } from '../../hooks/useProperFocus'; +import { useMergedRefs } from '../../hooks/useMergedRefs'; const backgroundColor: { [key in ButtonVariant | `${ButtonVariant}Disabled`]?: string; @@ -134,13 +137,19 @@ type ButtonProps = ComponentPropsWithoutRef & { variant?: ButtonVariant; bounce?: boolean; children?: ReactNode; + focused?: boolean; }; type ButtonVariant = 'normal' | 'primary' | 'bare' | 'menu' | 'menuSelected'; export const Button = forwardRef( - (props, ref) => { - const { children, variant = 'normal', bounce = true, ...restProps } = props; + (props, buttonRef) => { + const { children, variant = 'normal', bounce = true, focused = false, ...restProps } = props; + + const ref = useRef(null); + useProperFocus(ref, focused); + + const mergedRef = useMergedRefs(ref, buttonRef); const variantWithDisabled: ButtonVariant | `${ButtonVariant}Disabled` = props.isDisabled ? `${variant}Disabled` : variant; @@ -174,7 +183,7 @@ export const Button = forwardRef( return ( onClose()} data-testid="cancel-button" + focused={focusedField === 'cancel'} > Cancel @@ -1750,6 +1751,7 @@ function NewTransaction({ style={{ padding: '4px 10px' }} onPress={onAdd} data-testid="add-button" + focused={focusedField === 'add'} > Add @@ -2108,10 +2110,10 @@ export const TransactionTable = forwardRef((props, ref) => { } }, [prevSplitsExpanded.current]); - const newNavigator = useTableNavigator(newTransactions, getFields); + const newNavigator = useTableNavigator(newTransactions, getFieldsNewTransaction); const tableNavigator = useTableNavigator( transactionsWithExpandedSplits, - getFields, + getFieldsTableTransaction, ); const shouldAdd = useRef(false); const latestState = useRef({ newTransactions, newNavigator, tableNavigator }); @@ -2176,7 +2178,8 @@ export const TransactionTable = forwardRef((props, ref) => { savePending.current = false; }, [newTransactions, props.transactions]); - function getFields(item) { + + function getFieldsNewTransaction(item){ let fields = [ 'select', 'date', @@ -2187,8 +2190,30 @@ export const TransactionTable = forwardRef((props, ref) => { 'debit', 'credit', 'cleared', + 'cancel', + 'add' ]; + return getFields(item, fields); + } + + function getFieldsTableTransaction(item){ + let fields = [ + 'select', + 'date', + 'account', + 'payee', + 'notes', + 'category', + 'debit', + 'credit', + 'cleared', + ]; + + return getFields(item, fields); + } + + function getFields(item, fields) { fields = item.is_child ? ['select', 'payee', 'notes', 'category', 'debit', 'credit'] : fields.filter( diff --git a/upcoming-release-notes/3571.md b/upcoming-release-notes/3571.md new file mode 100644 index 00000000000..244655708a7 --- /dev/null +++ b/upcoming-release-notes/3571.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [The-Firexx] +--- + +Fixes focus ring getting "stuck" on last column of /accounts/budgeted screen when creating a new transaction. \ No newline at end of file From fa939a66faaecd585b95923f7b82ecbbd1ce3476 Mon Sep 17 00:00:00 2001 From: thefirex Date: Fri, 4 Oct 2024 22:48:26 +0100 Subject: [PATCH 2/4] Fix lint problems with the previous commit --- .../src/components/common/Button2.tsx | 13 +++++++++---- .../transactions/TransactionsTable.jsx | 17 ++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/desktop-client/src/components/common/Button2.tsx b/packages/desktop-client/src/components/common/Button2.tsx index 275c6924746..75cda991827 100644 --- a/packages/desktop-client/src/components/common/Button2.tsx +++ b/packages/desktop-client/src/components/common/Button2.tsx @@ -12,12 +12,12 @@ import { import { css } from 'glamor'; +import { useMergedRefs } from '../../hooks/useMergedRefs'; +import { useProperFocus } from '../../hooks/useProperFocus'; import { AnimatedLoading } from '../../icons/AnimatedLoading'; import { type CSSProperties, styles, theme } from '../../style'; import { View } from './View'; -import { useProperFocus } from '../../hooks/useProperFocus'; -import { useMergedRefs } from '../../hooks/useMergedRefs'; const backgroundColor: { [key in ButtonVariant | `${ButtonVariant}Disabled`]?: string; @@ -144,11 +144,16 @@ type ButtonVariant = 'normal' | 'primary' | 'bare' | 'menu' | 'menuSelected'; export const Button = forwardRef( (props, buttonRef) => { - const { children, variant = 'normal', bounce = true, focused = false, ...restProps } = props; + const { + children, + variant = 'normal', + bounce = true, + focused = false, + ...restProps + } = props; const ref = useRef(null); useProperFocus(ref, focused); - const mergedRef = useMergedRefs(ref, buttonRef); const variantWithDisabled: ButtonVariant | `${ButtonVariant}Disabled` = diff --git a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx index eb52fea9d38..587aa2bc3d7 100644 --- a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx +++ b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx @@ -2110,7 +2110,11 @@ export const TransactionTable = forwardRef((props, ref) => { } }, [prevSplitsExpanded.current]); - const newNavigator = useTableNavigator(newTransactions, getFieldsNewTransaction); + const newNavigator = useTableNavigator( + newTransactions, + getFieldsNewTransaction, + ); + const tableNavigator = useTableNavigator( transactionsWithExpandedSplits, getFieldsTableTransaction, @@ -2178,9 +2182,8 @@ export const TransactionTable = forwardRef((props, ref) => { savePending.current = false; }, [newTransactions, props.transactions]); - - function getFieldsNewTransaction(item){ - let fields = [ + function getFieldsNewTransaction(item) { + const fields = [ 'select', 'date', 'account', @@ -2191,14 +2194,14 @@ export const TransactionTable = forwardRef((props, ref) => { 'credit', 'cleared', 'cancel', - 'add' + 'add', ]; return getFields(item, fields); } - function getFieldsTableTransaction(item){ - let fields = [ + function getFieldsTableTransaction(item) { + const fields = [ 'select', 'date', 'account', From a6f46ada0e2f6277ba075c395e6b881227677ee3 Mon Sep 17 00:00:00 2001 From: thefirex Date: Sun, 13 Oct 2024 23:14:22 +0100 Subject: [PATCH 3/4] Changed the way the hook is made to the cancel and add button, removing the need to change Button2 --- .../src/components/common/Button2.tsx | 20 +++---------------- .../transactions/TransactionsTable.jsx | 10 ++++++++-- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/packages/desktop-client/src/components/common/Button2.tsx b/packages/desktop-client/src/components/common/Button2.tsx index 75cda991827..694429d573c 100644 --- a/packages/desktop-client/src/components/common/Button2.tsx +++ b/packages/desktop-client/src/components/common/Button2.tsx @@ -1,7 +1,6 @@ import React, { forwardRef, useMemo, - useRef, type ComponentPropsWithoutRef, type ReactNode, } from 'react'; @@ -12,8 +11,6 @@ import { import { css } from 'glamor'; -import { useMergedRefs } from '../../hooks/useMergedRefs'; -import { useProperFocus } from '../../hooks/useProperFocus'; import { AnimatedLoading } from '../../icons/AnimatedLoading'; import { type CSSProperties, styles, theme } from '../../style'; @@ -137,24 +134,13 @@ type ButtonProps = ComponentPropsWithoutRef & { variant?: ButtonVariant; bounce?: boolean; children?: ReactNode; - focused?: boolean; }; type ButtonVariant = 'normal' | 'primary' | 'bare' | 'menu' | 'menuSelected'; export const Button = forwardRef( - (props, buttonRef) => { - const { - children, - variant = 'normal', - bounce = true, - focused = false, - ...restProps - } = props; - - const ref = useRef(null); - useProperFocus(ref, focused); - const mergedRef = useMergedRefs(ref, buttonRef); + (props, ref) => { + const { children, variant = 'normal', bounce = true, ...restProps } = props; const variantWithDisabled: ButtonVariant | `${ButtonVariant}Disabled` = props.isDisabled ? `${variant}Disabled` : variant; @@ -188,7 +174,7 @@ export const Button = forwardRef( return ( t.amount === 0); + const addRef = useRef(null); + useProperFocus(addRef, focusedField === 'add'); + const cancelRef = useRef(null); + useProperFocus(cancelRef, focusedField === 'cancel'); + return ( onClose()} data-testid="cancel-button" - focused={focusedField === 'cancel'} + ref={cancelRef} > Cancel @@ -1751,7 +1757,7 @@ function NewTransaction({ style={{ padding: '4px 10px' }} onPress={onAdd} data-testid="add-button" - focused={focusedField === 'add'} + ref={addRef} > Add From 32b74b624430f1d42748145a8c7ce150bf06ce36 Mon Sep 17 00:00:00 2001 From: thefirex Date: Mon, 11 Nov 2024 19:29:40 +0000 Subject: [PATCH 4/4] Changed the name of variables as mentioned in PR --- .../components/transactions/TransactionsTable.jsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx index 5665224c742..665dbff691b 100644 --- a/packages/desktop-client/src/components/transactions/TransactionsTable.jsx +++ b/packages/desktop-client/src/components/transactions/TransactionsTable.jsx @@ -1671,10 +1671,10 @@ function NewTransaction({ ); const emptyChildTransactions = childTransactions.filter(t => t.amount === 0); - const addRef = useRef(null); - useProperFocus(addRef, focusedField === 'add'); - const cancelRef = useRef(null); - useProperFocus(cancelRef, focusedField === 'cancel'); + const addButtonRef = useRef(null); + useProperFocus(addButtonRef, focusedField === 'add'); + const cancelButtonRef = useRef(null); + useProperFocus(cancelButtonRef, focusedField === 'cancel'); return ( onClose()} data-testid="cancel-button" - ref={cancelRef} + ref={cancelButtonRef} > Cancel @@ -1757,7 +1757,7 @@ function NewTransaction({ style={{ padding: '4px 10px' }} onPress={onAdd} data-testid="add-button" - ref={addRef} + ref={addButtonRef} > Add