Skip to content

Commit

Permalink
Merge branch 'main' into feat/rocketchat-icon
Browse files Browse the repository at this point in the history
  • Loading branch information
dougfabris authored Sep 15, 2023
2 parents 77e1e8f + 2c25e75 commit 976232f
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 139 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-students-sin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/fuselage": patch
---

Memory leak in `PaginatedMultiSelect`
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,26 @@ import {
useMutableCallback,
useResizeObserver,
} from '@rocket.chat/fuselage-hooks';
import type {
SyntheticEvent,
ComponentProps,
Ref,
ReactElement,
FormEvent,
import React, {
type ComponentProps,
type ReactElement,
useState,
useRef,
} from 'react';
import React, { useState, useRef, useCallback, forwardRef, memo } from 'react';

import { prevent } from '../../helpers/prevent';
import AnimatedVisibility from '../AnimatedVisibility';
import Box from '../Box';
import Chip from '../Chip';
import Flex from '../Flex';
import { Icon } from '../Icon';
import { InputBox } from '../InputBox';
import Margins from '../Margins';
import { useVisible } from '../Options/useVisible';
import { OptionsPaginated } from '../OptionsPaginated';
import Position from '../Position';
import SelectAddon from '../Select/SelectAddon';
import SelectFocus from '../Select/SelectFocus';

const SelectedOptions = memo<ComponentProps<typeof Chip>>((props) => (
<Chip maxWidth='x150' withTruncatedText {...props} />
));

export type PaginatedMultiSelectOption = {
value: string | number;
label: string;
Expand All @@ -51,7 +44,7 @@ type PaginatedMultiSelectProps = Omit<
anchor?: any;
};

export const PaginatedMultiSelect = ({
const PaginatedMultiSelect = ({
withTitle,
value,
filter,
Expand All @@ -61,7 +54,7 @@ export const PaginatedMultiSelect = ({
anchor: Anchor = SelectFocus,
onChange = () => {},
placeholder,
renderOptions: _Options = OptionsPaginated,
renderOptions: OptionsComponent = OptionsPaginated,
endReached,
...props
}: PaginatedMultiSelectProps) => {
Expand All @@ -73,19 +66,6 @@ export const PaginatedMultiSelect = ({
currentValue.some((opt) => opt.value === option.value)
);

const internalChanged = (option: PaginatedMultiSelectOption) => {
if (currentValue.some((opt) => opt.value === option.value)) {
const newValue = currentValue.filter((opt) => opt.value !== option.value);

setInternalValue(newValue);
return onChange(newValue);
}

const newValue = [...currentValue, option];
setInternalValue(newValue);
return onChange(newValue);
};

const [visible, hide, show] = useVisible();

const ref = useRef<HTMLInputElement>(null);
Expand All @@ -101,13 +81,32 @@ export const PaginatedMultiSelect = ({
}
});

const handleMouseDown = useMutableCallback(
(option: PaginatedMultiSelectOption) => (e: SyntheticEvent) => {
prevent(e);
internalChanged(option);
return false;
const addOption = (value: unknown) => {
const option = options.find((opt) => opt.value === value);
if (!option) {
return;
}
);

const newValue = [...currentValue, option];
setInternalValue(newValue);
onChange(newValue);
};

const removeOption = (value: unknown) => {
const newValue = currentValue.filter((opt) => opt.value !== value);

setInternalValue(newValue);
onChange(newValue);
};

const toggleOption = (value: unknown) => {
if (currentValue.some((opt) => opt.value === value)) {
removeOption(value);
return;
}

addOption(value);
};

return (
<Box
Expand Down Expand Up @@ -143,17 +142,21 @@ export const PaginatedMultiSelect = ({
children={placeholder ?? null}
/>

{selectedOptions.map((option, index: number) => (
<SelectedOptions
{...(withTitle && {
title: option.label,
})}
{selectedOptions.map(({ value, label }, index) => (
<Chip
key={value ?? index}
maxWidth='x150'
withTruncatedText
title={withTitle ? label : undefined}
tabIndex={-1}
role='option'
key={index}
onMouseDown={handleMouseDown(option)}
children={option.label}
/>
onClick={(e) => {
prevent(e);
removeOption(value);
}}
>
{label}
</Chip>
))}
</Margins>
</Box>
Expand All @@ -179,8 +182,7 @@ export const PaginatedMultiSelect = ({
</Flex.Item>
<AnimatedVisibility visibility={visible}>
<Position anchor={containerRef}>
<_Options
withTitle={withTitle}
<OptionsComponent
width={borderBoxSize.inlineSize}
onMouseDown={prevent}
multiple
Expand All @@ -189,65 +191,14 @@ export const PaginatedMultiSelect = ({
options={options}
cursor={-1}
endReached={endReached}
onSelect={([value, label]) =>
internalChanged({ value: value as string, label })
}
onSelect={([value]) => {
toggleOption(value);
}}
/>
</Position>
</AnimatedVisibility>
</Box>
);
};

type PaginatedMultiSelectFilteredProps = Omit<
PaginatedMultiSelectProps,
'filter'
> & {
filter?: string;
setFilter?: (value: string) => void;
};

export const PaginatedMultiSelectFiltered = ({
filter,
setFilter,
options,
placeholder,
...props
}: PaginatedMultiSelectFilteredProps) => {
// eslint-disable-next-line react-hooks/exhaustive-deps
const anchor = useCallback(
forwardRef(
(
{
children: _children,
filter,
...props
}: ComponentProps<typeof InputBox>,
ref: Ref<HTMLInputElement>
) => (
<Flex.Item grow={1}>
<InputBox.Input
ref={ref}
placeholder={placeholder}
value={filter}
onInput={(e: FormEvent<HTMLInputElement>) =>
setFilter?.(e.currentTarget.value)
}
{...props}
rcx-input-box--undecorated
/>
</Flex.Item>
)
),
[]
);

return (
<PaginatedMultiSelect
filter={filter}
options={options}
{...props}
anchor={anchor}
/>
);
};
export default PaginatedMultiSelect;
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export default {
},
} as ComponentMeta<typeof PaginatedMultiSelectFiltered>;

const Template: ComponentStory<typeof PaginatedMultiSelectFiltered> = (
args
) => <PaginatedMultiSelectFiltered {...args} />;

export const Normal: ComponentStory<typeof PaginatedMultiSelectFiltered> = (
args
) => {
Expand All @@ -33,16 +37,12 @@ export const Normal: ComponentStory<typeof PaginatedMultiSelectFiltered> = (
);
};

export const Errored: ComponentStory<typeof PaginatedMultiSelectFiltered> = (
args
) => <PaginatedMultiSelectFiltered {...args} />;
export const Errored = Template.bind({});
Errored.args = {
error: true,
};

export const Disabled: ComponentStory<typeof PaginatedMultiSelectFiltered> = (
args
) => <PaginatedMultiSelectFiltered {...args} />;
export const Disabled = Template.bind({});
Disabled.args = {
disabled: true,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import type { ComponentProps, Ref, FormEvent, ReactElement } from 'react';
import React, { useCallback, forwardRef } from 'react';

import type Box from '../Box';
import Flex from '../Flex';
import { InputBox } from '../InputBox';
import { type OptionsPaginated } from '../OptionsPaginated';
import PaginatedMultiSelect, {
type PaginatedMultiSelectOption,
} from './PaginatedMultiSelect';

type PaginatedMultiSelectFilteredProps = Omit<
ComponentProps<typeof Box>,
'onChange' | 'value' | 'filter'
> & {
error?: boolean;
options: PaginatedMultiSelectOption[];
withTitle?: boolean;
placeholder?: string;
endReached?: (start?: number, end?: number) => void;
value?: PaginatedMultiSelectOption[];
onChange: (values: PaginatedMultiSelectOption[]) => void;
renderOptions?: (
props: ComponentProps<typeof OptionsPaginated>
) => ReactElement | null;
anchor?: any;
filter?: string;
setFilter?: (value: string) => void;
};

export const PaginatedMultiSelectFiltered = ({
filter,
setFilter,
options,
placeholder,
...props
}: PaginatedMultiSelectFilteredProps) => {
// eslint-disable-next-line react-hooks/exhaustive-deps
const anchor = useCallback(
forwardRef(
(
{
children: _children,
filter,
...props
}: ComponentProps<typeof InputBox>,
ref: Ref<HTMLInputElement>
) => (
<Flex.Item grow={1}>
<InputBox.Input
ref={ref}
placeholder={placeholder}
value={filter}
onInput={(e: FormEvent<HTMLInputElement>) =>
setFilter?.(e.currentTarget.value)
}
{...props}
rcx-input-box--undecorated
/>
</Flex.Item>
)
),
[]
);

return (
<PaginatedMultiSelect
filter={filter}
options={options}
{...props}
anchor={anchor}
/>
);
};
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// @ts-nocheck
import {
useMutableCallback,
useResizeObserver,
} from '@rocket.chat/fuselage-hooks';
import type { SyntheticEvent, ElementType } from 'react';
import React, { useState, useRef, useMemo, useEffect } from 'react';
import React, { type ElementType, useState, useRef, useMemo } from 'react';

import type { SelectProps } from '..';
import { type SelectProps } from '..';
import { prevent } from '../../helpers/prevent';
import AnimatedVisibility from '../AnimatedVisibility';
import Box from '../Box';
import { Icon } from '../Icon';
Expand All @@ -30,24 +29,6 @@ export type PaginatedSelectProps = Omit<SelectProps, 'options'> & {
setFilter?: (value: string | undefined | number) => void;
};

const prevent = (e: SyntheticEvent) => {
e.preventDefault();
e.stopPropagation();
e.nativeEvent.stopImmediatePropagation();
};

const useDidUpdate = (func: string[]) => {
const didMount = useRef(false);
const fn = useMutableCallback(func as any);

useEffect(() => {
if (didMount.current) {
fn();
}
didMount.current = true;
}, [fn]);
};

export const PaginatedSelect = ({
value,
withTitle,
Expand Down Expand Up @@ -81,8 +62,6 @@ export const PaginatedSelect = ({

const { ref: containerRef, borderBoxSize } = useResizeObserver();

useDidUpdate([filter, internalValue]);

const valueLabel = option?.label;

const visibleText =
Expand Down
Loading

0 comments on commit 976232f

Please sign in to comment.