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.

Sign up for GitHub

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

Jump to bottom

♻️ (tooltip) refactoring to react-aria (vol.3) #2631

Merged
merged 17 commits into from
May 7, 2024
Merged

♻️ (tooltip) refactoring to react-aria (vol.3) #2631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
c842b90
:recycle: (tooltip) refactoring to react-aria (vol.3)
MatissJanis Apr 18, 2024
8834f02
Release notes
MatissJanis Apr 18, 2024
28d909f
Merge branch 'master' into matiss/tooltip-filters
MatissJanis Apr 22, 2024
3474c12
Patch VRT
MatissJanis Apr 22, 2024
e0401b0
Merge branch 'master' into matiss/tooltip-filters
MatissJanis Apr 22, 2024
e6e08b1
Merge branch 'master' into matiss/tooltip-filters
MatissJanis Apr 25, 2024
3500041
Add scrollbar style
MatissJanis Apr 25, 2024
5a812db
Merge branch 'master' into matiss/tooltip-filters
MatissJanis Apr 26, 2024
2990c6d
Patch build failing
MatissJanis Apr 26, 2024
4d41d83
Merge branch 'master' into matiss/tooltip-filters
MatissJanis Apr 27, 2024
7787d38
Scrollbar update
MatissJanis Apr 27, 2024
12dd06c
Potential scrollbar patch
MatissJanis Apr 28, 2024
6135671
Localize scrollbar patch in popovers & autocomplete
MatissJanis Apr 29, 2024
4ddf446
Feedback: covert to glamor
MatissJanis May 2, 2024
cd0fbfa
Fix scrollbar style
joel-jeremy May 6, 2024
82beb0f
Apply darkScrollbar to embedded autocomplete
joel-jeremy May 6, 2024
f6e2a55
Feedback: patch bulk edit input field
MatissJanis May 6, 2024
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
2 changes: 1 addition & 1 deletion packages/desktop-client/e2e/transactions.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ test.describe('Transactions', () => {
await expect(datepicker).toMatchThemeScreenshots();

// Select "is xxxxx"
await datepicker.getByRole('button', { name: '20' }).click();
await datepicker.getByText('20', { exact: true }).click();
await filterTooltip.applyButton.click();

// Assert that there are no transactions
Expand Down
228 changes: 114 additions & 114 deletions packages/desktop-client/src/components/autocomplete/Autocomplete.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ import { css } from 'glamor';

import { SvgRemove } from '../../icons/v2';
import { useResponsive } from '../../ResponsiveProvider';
import { theme, type CSSProperties, styles } from '../../style';
import { theme, styles } from '../../style';
import { Button } from '../common/Button';
import { Input } from '../common/Input';
import { Popover } from '../common/Popover';
import { View } from '../common/View';
import { Tooltip } from '../tooltips';

type CommonAutocompleteProps<T extends Item> = {
focused?: boolean;
Expand All @@ -31,8 +31,6 @@ type CommonAutocompleteProps<T extends Item> = {
onChange?: (value: string) => void;
};
suggestions?: T[];
tooltipStyle?: CSSProperties;
tooltipProps?: ComponentProps<typeof Tooltip>;
renderInput?: (props: ComponentProps<typeof Input>) => ReactNode;
renderItems?: (
items: T[],
Expand Down Expand Up @@ -214,8 +212,6 @@ function SingleAutocomplete<T extends Item>({
labelProps = {},
inputProps = {},
suggestions,
tooltipStyle,
tooltipProps,
renderInput = defaultRenderInput,
renderItems = defaultRenderItems,
itemToString = defaultItemToString,
Expand Down Expand Up @@ -253,6 +249,8 @@ function SingleAutocomplete<T extends Item>({
onClose?.();
};

const triggerRef = useRef(null);

const { isNarrowWidth } = useResponsive();
const narrowInputStyle = isNarrowWidth
? {
Expand Down Expand Up @@ -442,115 +440,122 @@ function SingleAutocomplete<T extends Item>({
// can't use a View here, but we can fake it be using the
// className
<div className={`view ${css({ display: 'flex' })}`} {...containerProps}>
{renderInput(
getInputProps({
focused,
...inputProps,
onFocus: e => {
inputProps.onFocus?.(e);

if (openOnFocus) {
open();
}
},
onBlur: e => {
// Should this be e.nativeEvent
e['preventDownshiftDefault'] = true;
inputProps.onBlur?.(e);

if (!closeOnBlur) return;

if (clearOnBlur) {
if (e.target.value === '') {
onSelect?.(null, e.target.value);
setSelectedItem(null);
close();
return;
<View ref={triggerRef} style={{ flexShrink: 0 }}>
{renderInput(
getInputProps({
focused,
...inputProps,
onFocus: e => {
inputProps.onFocus?.(e);

if (openOnFocus) {
open();
}
},
onBlur: e => {
// Should this be e.nativeEvent
e['preventDownshiftDefault'] = true;
inputProps.onBlur?.(e);

if (!closeOnBlur) return;

if (clearOnBlur) {
if (e.target.value === '') {
onSelect?.(null, e.target.value);
setSelectedItem(null);
close();
return;
}

// If not using table behavior, reset the input on blur. Tables
// handle saving the value on blur.
const value = selectedItem ? getItemId(selectedItem) : null;

resetState(value);
} else {
close();
}
},
onKeyDown: (e: KeyboardEvent<HTMLInputElement>) => {
const { onKeyDown } = inputProps || {};

// If the dropdown is open, an item is highlighted, and the user
// pressed enter, always capture that and handle it ourselves
if (isOpen) {
if (e.key === 'Enter') {
if (highlightedIndex != null) {
if (
inst.lastChangeType ===
Downshift.stateChangeTypes.itemMouseEnter
) {
// If the last thing the user did was hover an item, intentionally
// ignore the default behavior of selecting the item. It's too
// common to accidentally hover an item and then save it
e.preventDefault();
} else {
// Otherwise, stop propagation so that the table navigator
// doesn't handle it
// If not using table behavior, reset the input on blur. Tables
// handle saving the value on blur.
const value = selectedItem ? getItemId(selectedItem) : null;

resetState(value);
} else {
close();
}
},
onKeyDown: (e: KeyboardEvent<HTMLInputElement>) => {
const { onKeyDown } = inputProps || {};

// If the dropdown is open, an item is highlighted, and the user
// pressed enter, always capture that and handle it ourselves
if (isOpen) {
if (e.key === 'Enter') {
if (highlightedIndex != null) {
if (
inst.lastChangeType ===
Downshift.stateChangeTypes.itemMouseEnter
) {
// If the last thing the user did was hover an item, intentionally
// ignore the default behavior of selecting the item. It's too
// common to accidentally hover an item and then save it
e.preventDefault();
} else {
// Otherwise, stop propagation so that the table navigator
// doesn't handle it
e.stopPropagation();
}
} else if (!strict) {
// Handle it ourselves
e.stopPropagation();
onSelect(value, (e.target as HTMLInputElement).value);
return onSelectAfter();
} else {
// No highlighted item, still allow the table to save the item
// as `null`, even though we're allowing the table to move
e.preventDefault();
onKeyDown?.(e);
}
} else if (!strict) {
// Handle it ourselves
e.stopPropagation();
onSelect(value, (e.target as HTMLInputElement).value);
return onSelectAfter();
} else {
// No highlighted item, still allow the table to save the item
// as `null`, even though we're allowing the table to move
} else if (shouldSaveFromKey(e)) {
e.preventDefault();
onKeyDown?.(e);
}
} else if (shouldSaveFromKey(e)) {
e.preventDefault();
onKeyDown?.(e);
}
}

// Handle escape ourselves
if (e.key === 'Escape') {
e.nativeEvent['preventDownshiftDefault'] = true;
// Handle escape ourselves
if (e.key === 'Escape') {
e.nativeEvent['preventDownshiftDefault'] = true;

if (!embedded) {
e.stopPropagation();
}
if (!embedded) {
e.stopPropagation();
}

fireUpdate(
onUpdate,
strict,
suggestions,
null,
getItemId(originalItem),
);

setValue(getItemName(originalItem));
setSelectedItem(findItem(strict, suggestions, originalItem));
setHighlightedIndex(null);
if (embedded) {
open();
} else {
close();
fireUpdate(
onUpdate,
strict,
suggestions,
null,
getItemId(originalItem),
);

setValue(getItemName(originalItem));
setSelectedItem(
findItem(strict, suggestions, originalItem),
);
setHighlightedIndex(null);
if (embedded) {
open();
} else {
close();
}
}
}
},
onChange: (e: ChangeEvent<HTMLInputElement>) => {
const { onChange } = inputProps || {};
onChange?.(e.target.value);
},
}),
)}
},
onChange: (e: ChangeEvent<HTMLInputElement>) => {
const { onChange } = inputProps || {};
onChange?.(e.target.value);
},
}),
)}
</View>
{isOpen &&
filtered.length > 0 &&
(embedded ? (
<View style={{ marginTop: 5 }} data-testid="autocomplete">
<View
style={{ ...styles.darkScrollbar, marginTop: 5 }}
data-testid="autocomplete"
>
{renderItems(
filtered,
getItemProps,
Expand All @@ -559,17 +564,20 @@ function SingleAutocomplete<T extends Item>({
)}
</View>
) : (
<Tooltip
position="bottom-stretch"
<Popover
triggerRef={triggerRef}
placement="bottom start"
offset={2}
isOpen={isOpen}
onOpenChange={close}
isNonModal
style={{
padding: 0,
...styles.darkScrollbar,
Copy link
Contributor

@joel-jeremy joel-jeremy May 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Instead of putting this in a style prop, can we try using glamor to pass this to className? Maybe the inline style prop doesn't support pseudo selectors. Another possible option is to just use the scrollbarWidth and scrollColor css props instead of the darkScrollbar.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using glamor is a good idea! Updated it now, let me know how it looks!

Copy link
Contributor

Choose a reason for hiding this comment

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

It still is not working, we need to check that the darkScrollbar is set in the children's container element.

backgroundColor: theme.menuAutoCompleteBackground,
color: theme.menuAutoCompleteText,
minWidth: 200,
...tooltipStyle,
width: triggerRef.current?.clientWidth,
}}
{...tooltipProps}
data-testid="autocomplete"
>
{renderItems(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can try wrapping this in a View and setting the scrollbar style there?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would you be open to try this out locally? I'm working blindly here without a way to reproduce.

Expand All @@ -578,7 +586,7 @@ function SingleAutocomplete<T extends Item>({
highlightedIndex,
inputValue,
)}
</Tooltip>
</Popover>
))}
</div>
)}
Expand Down Expand Up @@ -626,13 +634,8 @@ function MultiAutocomplete<T extends Item>({
...props
}: MultiAutocompleteProps<T>) {
const [focused, setFocused] = useState(false);
const lastSelectedItems = useRef<typeof selectedItems>();
const selectedItemIds = selectedItems.map(getItemId);

useEffect(() => {
lastSelectedItems.current = selectedItems;
});

function onRemoveItem(id: T['id']) {
const items = selectedItemIds.filter(i => i !== id);
onSelect(items);
Expand Down Expand Up @@ -669,9 +672,6 @@ function MultiAutocomplete<T extends Item>({
onSelect={onAddItem}
highlightFirst
strict={strict}
tooltipProps={{
forceLayout: lastSelectedItems.current !== selectedItems,
}}
renderInput={inputProps => (
<View
style={{
Expand Down
14 changes: 9 additions & 5 deletions packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ function CategoryList({
}: CategoryListProps) {
let lastGroup: string | undefined | null = null;

const filteredItems = useMemo(
() =>
showHiddenItems
? items
: items.filter(item => !item.hidden && !item.group?.hidden),
[showHiddenItems, items],
);

return (
<View>
<View
Expand All @@ -79,7 +87,7 @@ function CategoryList({
...(!embedded && { maxHeight: 175 }),
}}
>
{items.map((item, idx) => {
{filteredItems.map((item, idx) => {
if (item.id === 'split') {
return renderSplitTransactionButton({
key: 'split',
Expand All @@ -89,10 +97,6 @@ function CategoryList({
});
}

if ((item.hidden || item.group?.hidden) && !showHiddenItems) {
return <Fragment key={item.id} />;
}

const showGroup = item.cat_group !== lastGroup;
const groupName = `${item.group?.name}${item.group?.hidden ? ' (hidden)' : ''}`;
lastGroup = item.cat_group;
Expand Down
4 changes: 2 additions & 2 deletions packages/desktop-client/src/components/common/Input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ export function Input({
)}`}
{...nativeProps}
onKeyDown={e => {
nativeProps.onKeyDown?.(e);

if (e.key === 'Enter' && onEnter) {
onEnter(e);
}

if (e.key === 'Escape' && onEscape) {
onEscape(e);
}

nativeProps.onKeyDown?.(e);
}}
onBlur={e => {
onUpdate?.(e.target.value);
Expand Down
Loading
Loading