Skip to content

Commit

Permalink
Larger mobile autocomplete fonts and paddings (#1900)
Browse files Browse the repository at this point in the history
* Larger mobile autocomplete fonts and paddings

* Release notes

* VRT + update tests

* Update tests

* Update data-highlighted and tests

* Use styles text

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Adjust Add Transaction padding + VRT updates

* Larger autocomplete text and divider

* Fix rebase

* Fix rebase

* Fix icons

* Adjust fonts

* Fix lint errors

* PR feedback

* VRT

* Update embedded autocomplete highlight hover color

* Refactor create payee button

* Embedded create payee button color

* Dummy change to re-run CI
  • Loading branch information
joel-jeremy authored Dec 1, 2023
1 parent c96782d commit 5d1f2d4
Show file tree
Hide file tree
Showing 16 changed files with 652 additions and 314 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion packages/desktop-client/e2e/transactions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ test.describe('Transactions', () => {
await expect(autocomplete).toMatchThemeScreenshots();

// Select the active item
await page.getByRole('button', { name: 'Clothing' }).click();
await page.getByTestId('Clothing-category-item').click();
await filterTooltip.applyButton.click();

// Assert that there are only clothing transactions
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, { type ComponentProps } from 'react';
import React, { Fragment, type ComponentProps, type ReactNode } from 'react';

import { css } from 'glamor';

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

import { theme } from '../../style';
import { type CSSProperties, theme } from '../../style';
import View from '../common/View';

import Autocomplete from './Autocomplete';
Expand All @@ -14,7 +15,8 @@ function AccountList({
getItemProps,
highlightedIndex,
embedded,
groupHeaderStyle,
renderAccountItemGroupHeader = defaultRenderAccountItemGroupHeader,
renderAccountItem = defaultRenderAccountItem,
}) {
let lastItem = null;

Expand Down Expand Up @@ -45,83 +47,43 @@ function AccountList({

return [
showGroup ? (
<div
key={group}
style={{
color: theme.menuAutoCompleteTextHeader,
padding: '4px 9px',
...groupHeaderStyle,
}}
data-testid="account-item-group"
>
{group}
</div>
<Fragment key={group}>
{renderAccountItemGroupHeader({ title: group })}
</Fragment>
) : null,
<div
// List each account up to a max
{...(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}
className={`${css([
{
backgroundColor:
highlightedIndex === idx
? theme.menuAutoCompleteBackgroundHover
: 'transparent',
padding: 4,
paddingLeft: 20,
borderRadius: embedded ? 4 : 0,
},
])}`}
data-testid={
'account-item' +
(highlightedIndex === idx ? '-highlighted' : '')
}
>
{item.name}
</div>,
<Fragment key={item.id}>
{renderAccountItem({
...(getItemProps ? getItemProps({ item }) : null),
item,
highlighted: highlightedIndex === idx,
embedded,
})}
</Fragment>,
];
})}
</View>
</View>
);
}

type AutoCompleteProps = {
type AccountAutoCompleteProps = {
embedded?: boolean;
includeClosedAccounts: boolean;
groupHeaderStyle?: boolean;
renderAccountItemGroupHeader?: (
props: AccountItemGroupHeaderProps,
) => ReactNode;
renderAccountItem?: (props: AccountItemProps) => ReactNode;
closeOnBlur?: boolean;
} & ComponentProps<typeof Autocomplete>;

export default function AccountAutocomplete({
embedded,
includeClosedAccounts = true,
groupHeaderStyle,
renderAccountItemGroupHeader,
renderAccountItem,
closeOnBlur,
...props
}: AutoCompleteProps) {
}: AccountAutoCompleteProps) {
let accounts = useCachedAccounts() || [];

//remove closed accounts if needed
Expand Down Expand Up @@ -151,10 +113,107 @@ export default function AccountAutocomplete({
getItemProps={getItemProps}
highlightedIndex={highlightedIndex}
embedded={embedded}
groupHeaderStyle={groupHeaderStyle}
renderAccountItemGroupHeader={renderAccountItemGroupHeader}
renderAccountItem={renderAccountItem}
/>
)}
{...props}
/>
);
}

type AccountItemGroupHeaderProps = {
title: string;
style?: CSSProperties;
};

export function AccountItemGroupHeader({
title,
style,
...props
}: AccountItemGroupHeaderProps) {
return (
<div
style={{
color: theme.menuAutoCompleteTextHeader,
padding: '4px 9px',
...style,
}}
data-testid={`${title}-account-item-group`}
{...props}
>
{title}
</div>
);
}

function defaultRenderAccountItemGroupHeader(
props: AccountItemGroupHeaderProps,
): ReactNode {
return <AccountItemGroupHeader {...props} />;
}

type AccountItemProps = {
item: AccountEntity;
className?: string;
style?: CSSProperties;
highlighted?: boolean;
embedded?: boolean;
};

export function AccountItem({
item,
className,
highlighted,
embedded,
...props
}: AccountItemProps) {
return (
<div
// List each account up to a max
// 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"
className={`${className} ${css([
{
backgroundColor: highlighted
? embedded
? theme.menuItemBackgroundHover
: theme.menuAutoCompleteBackgroundHover
: 'transparent',
padding: 4,
paddingLeft: 20,
borderRadius: embedded ? 4 : 0,
},
])}`}
data-testid={`${item.name}-account-item`}
data-highlighted={highlighted || undefined}
{...props}
>
{item.name}
</div>
);
}

function defaultRenderAccountItem(props: AccountItemProps): ReactNode {
return <AccountItem {...props} />;
}
Loading

0 comments on commit 5d1f2d4

Please sign in to comment.