Skip to content

Commit

Permalink
Initial port of react native edit transaction view (actualbudget#1340)
Browse files Browse the repository at this point in the history
* Set `role="button"` on downshift autocomplete items
This avoids content observation behavior in WebKit on touch devices that delays the onClick event (and therefore reaction to user input).
* Disable split transaction editing for now
  • Loading branch information
Cldfire authored Aug 8, 2023
1 parent 350fccd commit ad503a6
Show file tree
Hide file tree
Showing 13 changed files with 1,562 additions and 145 deletions.
32 changes: 31 additions & 1 deletion packages/desktop-client/src/components/FinancesApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { NarrowAlternate, WideComponent } from './responsive';
import PostsOfflineNotification from './schedules/PostsOfflineNotification';
import Settings from './settings';
import Titlebar, { TitlebarProvider } from './Titlebar';
import { TransactionEdit } from './transactions/MobileTransaction';

function NarrowNotSupported({
redirectTo = '/budget',
Expand All @@ -61,6 +62,17 @@ function NarrowNotSupported({
return isNarrowWidth ? null : children;
}

function WideNotSupported({ children, redirectTo = '/budget' }) {
const { isNarrowWidth } = useResponsive();
const navigate = useNavigate();
useEffect(() => {
if (!isNarrowWidth) {
navigate(redirectTo);
}
}, [isNarrowWidth, navigate, redirectTo]);
return isNarrowWidth ? children : null;
}

function StackedRoutesInner({ location }) {
return (
<Routes location={location}>
Expand Down Expand Up @@ -147,12 +159,30 @@ function StackedRoutesInner({ location }) {
}
/>

<Route path="/accounts" element={<NarrowAlternate name="Accounts" />} />

<Route
path="/accounts/:id"
element={<NarrowAlternate name="Account" />}
/>

<Route path="/accounts" element={<NarrowAlternate name="Accounts" />} />
<Route
path="/accounts/:id/transactions/:transactionId"
element={
<WideNotSupported>
<TransactionEdit />
</WideNotSupported>
}
/>

<Route
path="/accounts/:id/transactions/new"
element={
<WideNotSupported>
<TransactionEdit />
</WideNotSupported>
}
/>
</Routes>
);
}
Expand Down
16 changes: 2 additions & 14 deletions packages/desktop-client/src/components/accounts/MobileAccount.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import * as queries from 'loot-core/src/client/queries';
import { pagedQuery } from 'loot-core/src/client/query-helpers';
import { send, listen } from 'loot-core/src/platform/client/fetch';
import {
getSplit,
isPreviewId,
ungroupTransactions,
} from 'loot-core/src/shared/transactions';
Expand Down Expand Up @@ -185,7 +184,6 @@ export default function Account(props) {
setSearchText(text);
};

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const onSelectTransaction = transaction => {
if (isPreviewId(transaction.id)) {
let parts = transaction.id.split('/');
Expand Down Expand Up @@ -214,17 +212,7 @@ export default function Account(props) {
},
);
} else {
let trans = [transaction];
if (transaction.parent_id || transaction.is_parent) {
let index = transactions.findIndex(
t => t.id === (transaction.parent_id || transaction.id),
);
trans = getSplit(transactions, index);
}

navigate('Transaction', {
transactions: trans,
});
navigate(`transactions/${transaction.id}`);
}
};

Expand Down Expand Up @@ -269,7 +257,7 @@ export default function Account(props) {
paged?.fetchNext();
}}
onSearch={onSearch}
onSelectTransaction={() => {}} // onSelectTransaction}
onSelectTransaction={onSelectTransaction}
/>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ export default function AccountDetails({
TODO: connect to an add transaction modal
Only left here but hidden for flex centering of the account name.
*/}
<Link to="transaction/new" style={{ visibility: 'hidden' }}>
<Link to="transactions/new">
<Button
type="bare"
style={{ justifyContent: 'center', width: LEFT_RIGHT_FLEX_WIDTH }}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
import React from 'react';

import { css } from 'glamor';

import { useCachedAccounts } from 'loot-core/src/client/data-hooks/accounts';

import { useResponsive } from '../../ResponsiveProvider';
import { colors } from '../../style';
import View from '../common/View';

import Autocomplete from './Autocomplete';

function AccountList({ items, getItemProps, highlightedIndex, embedded }) {
function AccountList({
items,
getItemProps,
highlightedIndex,
embedded,
groupHeaderStyle,
}) {
let lastItem = null;
const { isNarrowWidth } = useResponsive();
const highlightedIndexColor = isNarrowWidth
? 'rgba(100, 100, 100, .15)'
: colors.n4;

return (
<View>
Expand Down Expand Up @@ -41,6 +54,7 @@ function AccountList({ items, getItemProps, highlightedIndex, embedded }) {
style={{
color: colors.y9,
padding: '4px 9px',
...groupHeaderStyle,
}}
data-testid="account-item-group"
>
Expand All @@ -49,14 +63,43 @@ function AccountList({ items, getItemProps, highlightedIndex, embedded }) {
) : null,
<div
{...(getItemProps ? getItemProps({ item }) : null)}
// Downshift calls `setTimeout(..., 250)` in the `onMouseMove`
// event handler they set on this element. When this code runs
// in WebKit on touch-enabled devices, taps on this element end
// up not triggering the `onClick` event (and therefore delaying
// response to user input) until after the `setTimeout` callback
// finishes executing. This is caused by content observation code
// that implements various strategies to prevent the user from
// accidentally clicking content that changed as a result of code
// run in the `onMouseMove` event.
//
// Long story short, we don't want any delay here between the user
// tapping and the resulting action being performed. It turns out
// there's some "fast path" logic that can be triggered in various
// ways to force WebKit to bail on the content observation process.
// One of those ways is setting `role="button"` (or a number of
// other aria roles) on the element, which is what we're doing here.
//
// ref:
// * https://github.com/WebKit/WebKit/blob/447d90b0c52b2951a69df78f06bb5e6b10262f4b/LayoutTests/fast/events/touch/ios/content-observation/400ms-hover-intent.html
// * https://github.com/WebKit/WebKit/blob/58956cf59ba01267644b5e8fe766efa7aa6f0c5c/Source/WebCore/page/ios/ContentChangeObserver.cpp
// * https://github.com/WebKit/WebKit/blob/58956cf59ba01267644b5e8fe766efa7aa6f0c5c/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm#L783
role="button"
key={item.id}
style={{
backgroundColor:
highlightedIndex === idx ? colors.n4 : 'transparent',
padding: 4,
paddingLeft: 20,
borderRadius: embedded ? 4 : 0,
}}
className={`${css([
{
backgroundColor:
highlightedIndex === idx
? highlightedIndexColor
: 'transparent',
padding: 4,
paddingLeft: 20,
borderRadius: embedded ? 4 : 0,
':active': {
backgroundColor: 'rgba(100, 100, 100, .25)',
},
},
])}`}
data-testid={
'account-item' +
(highlightedIndex === idx ? '-highlighted' : '')
Expand All @@ -74,6 +117,8 @@ function AccountList({ items, getItemProps, highlightedIndex, embedded }) {
export default function AccountAutocomplete({
embedded,
includeClosedAccounts = true,
groupHeaderStyle,
closeOnBlur,
...props
}) {
let accounts = useCachedAccounts() || [];
Expand All @@ -97,13 +142,15 @@ export default function AccountAutocomplete({
strict={true}
highlightFirst={true}
embedded={embedded}
closeOnBlur={closeOnBlur}
suggestions={accounts}
renderItems={(items, getItemProps, highlightedIndex) => (
<AccountList
items={items}
getItemProps={getItemProps}
highlightedIndex={highlightedIndex}
embedded={embedded}
groupHeaderStyle={groupHeaderStyle}
/>
)}
{...props}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,28 @@ function defaultRenderItems(items, getItemProps, highlightedIndex) {
return (
<div
{...getItemProps({ item })}
// Downshift calls `setTimeout(..., 250)` in the `onMouseMove`
// event handler they set on this element. When this code runs
// in WebKit on touch-enabled devices, taps on this element end
// up not triggering the `onClick` event (and therefore delaying
// response to user input) until after the `setTimeout` callback
// finishes executing. This is caused by content observation code
// that implements various strategies to prevent the user from
// accidentally clicking content that changed as a result of code
// run in the `onMouseMove` event.
//
// Long story short, we don't want any delay here between the user
// tapping and the resulting action being performed. It turns out
// there's some "fast path" logic that can be triggered in various
// ways to force WebKit to bail on the content observation process.
// One of those ways is setting `role="button"` (or a number of
// other aria roles) on the element, which is what we're doing here.
//
// ref:
// * https://github.com/WebKit/WebKit/blob/447d90b0c52b2951a69df78f06bb5e6b10262f4b/LayoutTests/fast/events/touch/ios/content-observation/400ms-hover-intent.html
// * https://github.com/WebKit/WebKit/blob/58956cf59ba01267644b5e8fe766efa7aa6f0c5c/Source/WebCore/page/ios/ContentChangeObserver.cpp
// * https://github.com/WebKit/WebKit/blob/58956cf59ba01267644b5e8fe766efa7aa6f0c5c/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm#L783
role="button"
key={name}
{...css({
padding: 5,
Expand Down Expand Up @@ -143,6 +165,7 @@ type SingleAutocompleteProps = {
strict?: boolean;
onSelect: (id: unknown, value: string) => void;
tableBehavior?: boolean;
closeOnBlur?: boolean;
value: unknown[];
isMulti?: boolean;
};
Expand All @@ -167,6 +190,7 @@ function SingleAutocomplete({
strict,
onSelect,
tableBehavior,
closeOnBlur = true,
value: initialValue,
isMulti = false,
}: SingleAutocompleteProps) {
Expand Down Expand Up @@ -377,6 +401,8 @@ function SingleAutocomplete({
e.preventDownshiftDefault = true;
inputProps.onBlur?.(e);

if (!closeOnBlur) return;

if (!tableBehavior) {
if (e.target.value === '') {
onSelect?.(null, e.target.value);
Expand Down
Loading

0 comments on commit ad503a6

Please sign in to comment.