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

feat(components): add option to delete input of text-input and lineage-filter input with x and sort text-input datalist options #631

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(components): add option to delete input of text-input and lineage-filter input with x and sort text-input datalist options #631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 34 additions & 0 deletions components/src/preact/lineageFilter/lineage-filter.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { type Meta, type StoryObj } from '@storybook/preact';
import { expect, fireEvent, fn, waitFor, within } from '@storybook/test';

import { LineageFilter, type LineageFilterProps } from './lineage-filter';
import { previewHandles } from '../../../.storybook/preview';
Expand Down Expand Up @@ -55,6 +56,39 @@ export const Default: StoryObj<LineageFilterProps> = {
),
};

export const WithInitialValue: StoryObj<LineageFilterProps> = {
...Default,
args: {
...Default.args,
initialValue: 'XCB',
},
play: async ({ canvasElement, step }) => {
const canvas = within(canvasElement);

const changedListenerMock = fn();
await step('Setup event listener mock', async () => {
canvasElement.addEventListener('gs-lineage-filter-changed', changedListenerMock);
});

await waitFor(() => {
const input = canvas.getByPlaceholderText('Enter lineage', { exact: false });
expect(input).toHaveValue('XCB');
});

await step('Remove initial value', async () => {
await fireEvent.click(canvas.getByRole('button', { name: '✕' }));

await expect(changedListenerMock).toHaveBeenCalledWith(
expect.objectContaining({
detail: {
host: undefined,
},
}),
);
});
},
};

export const WithNoLapisField: StoryObj<LineageFilterProps> = {
...Default,
args: {
Expand Down
42 changes: 32 additions & 10 deletions components/src/preact/lineageFilter/lineage-filter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type FunctionComponent } from 'preact';
import { useContext, useRef } from 'preact/hooks';
import { useContext, useRef, useState } from 'preact/hooks';
import z from 'zod';

import { fetchLineageAutocompleteList } from './fetchLineageAutocompleteList';
Expand Down Expand Up @@ -45,6 +45,9 @@ const LineageFilterInner: FunctionComponent<LineageFilterInnerProps> = ({

const inputRef = useRef<HTMLInputElement>(null);

const [hasInput, setHasInput] = useState<boolean>(!!initialValue);
const [inputValue, setInputValue] = useState(initialValue || '');
Comment on lines +48 to +49
Copy link
Collaborator

Choose a reason for hiding this comment

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

hasInput is redundant - it can be computed from inputValue

Suggested change
const [hasInput, setHasInput] = useState<boolean>(!!initialValue);
const [inputValue, setInputValue] = useState(initialValue || '');
const [inputValue, setInputValue] = useState(initialValue || '');
const hasInput = inputValue.length > 0;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const [inputValue, setInputValue] = useState(initialValue || '');
const [inputValue, setInputValue] = useState(initialValue ?? '');


const { data, error, isLoading } = useQuery(
() => fetchLineageAutocompleteList(lapis, lapisField),
[lapisField, lapis],
Expand Down Expand Up @@ -73,6 +76,8 @@ const LineageFilterInner: FunctionComponent<LineageFilterInnerProps> = ({
composed: true,
}),
);
setHasInput(value !== undefined);
setInputValue(value || '');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this to the first line of this function - then you can even delete invalid input.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
setInputValue(value || '');
setInputValue(value ?? '');

}
};

Expand All @@ -85,15 +90,32 @@ const LineageFilterInner: FunctionComponent<LineageFilterInnerProps> = ({

return (
<>
<input
type='text'
class='input input-bordered w-full'
placeholder={placeholderText !== undefined ? placeholderText : lapisField}
onInput={onInput}
ref={inputRef}
list={lapisField}
value={initialValue}
/>
<div className='relative w-full'>
<input
type='text'
class='input input-bordered w-full pr-10'
placeholder={placeholderText !== undefined ? placeholderText : lapisField}
onInput={onInput}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
onInput={onInput}
onInput={() => onInput(inputRef.current?.value === '' ? undefined : inputRef.current?.value)}

And change line 68 from

    const onInput = () => {
        const value = inputRef.current?.value === '' ? undefined : inputRef.current?.value;

to

    const onInput = (value?: string) => {

ref={inputRef}
list={lapisField}
value={inputValue}
/>
{hasInput && (
<button
type='button'
name='✕'
className='absolute top-1/2 right-2 transform -translate-y-1/2 text-gray-500 hover:text-gray-700'
Copy link
Collaborator

Choose a reason for hiding this comment

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

See https://daisyui.com/components/input/#text-input-with-icon-inside for a better way how to get the icon inside the "input field"

onClick={() => {
if (inputRef.current) {
inputRef.current.value = '';
onInput();
}
}}
Comment on lines +108 to +113
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like the side effects of this - I' propose to implement it as:

Suggested change
onClick={() => {
if (inputRef.current) {
inputRef.current.value = '';
onInput();
}
}}
onClick={() => onInput(undefined)}

(see other comments for how to adapt the rest)

>
✕
</button>
)}
</div>
<datalist id={lapisField}>
{data.map((item) => (
<option value={item} key={item} />
Expand Down
2 changes: 1 addition & 1 deletion components/src/preact/textInput/fetchAutocompleteList.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ export async function fetchAutocompleteList(lapis: string, field: string, signal

const data = (await fetchAggregatedOperator.evaluate(lapis, signal)).content;

return data.map((item) => item[field]);
return data.map((item) => item[field]).sort();
}
21 changes: 19 additions & 2 deletions components/src/preact/textInput/text-input.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type Meta, type StoryObj } from '@storybook/preact';
import { expect, waitFor, within } from '@storybook/test';
import { expect, fireEvent, fn, waitFor, within } from '@storybook/test';

import data from './__mockData__/aggregated_hosts.json';
import { TextInput, type TextInputProps } from './text-input';
Expand Down Expand Up @@ -85,13 +85,30 @@ export const WithInitialValue: StoryObj<TextInputProps> = {
...Default.args,
initialValue: 'Homo sapiens',
},
play: async ({ canvasElement }) => {
play: async ({ canvasElement, step }) => {
const canvas = within(canvasElement);

const changedListenerMock = fn();
await step('Setup event listener mock', async () => {
canvasElement.addEventListener('gs-text-input-changed', changedListenerMock);
});

await waitFor(() => {
const input = canvas.getByPlaceholderText('Enter a host name', { exact: false });
expect(input).toHaveValue('Homo sapiens');
});

await step('Remove initial value', async () => {
await fireEvent.click(canvas.getByRole('button', { name: '✕' }));

await expect(changedListenerMock).toHaveBeenCalledWith(
expect.objectContaining({
detail: {
host: undefined,
},
}),
);
});
},
};

Expand Down
42 changes: 32 additions & 10 deletions components/src/preact/textInput/text-input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type FunctionComponent } from 'preact';
import { useContext, useRef } from 'preact/hooks';
import { useContext, useRef, useState } from 'preact/hooks';
import z from 'zod';

import { fetchAutocompleteList } from './fetchAutocompleteList';
Expand Down Expand Up @@ -41,6 +41,9 @@ const TextInputInner: FunctionComponent<TextInputInnerProps> = ({ lapisField, pl

const inputRef = useRef<HTMLInputElement>(null);

const [hasInput, setHasInput] = useState<boolean>(!!initialValue);
const [inputValue, setInputValue] = useState(initialValue || '');

const { data, error, isLoading } = useQuery(() => fetchAutocompleteList(lapis, lapisField), [lapisField, lapis]);

if (isLoading) {
Expand All @@ -66,6 +69,8 @@ const TextInputInner: FunctionComponent<TextInputInnerProps> = ({ lapisField, pl
composed: true,
}),
);
setHasInput(value !== undefined);
setInputValue(value || '');
}
};

Expand All @@ -78,15 +83,32 @@ const TextInputInner: FunctionComponent<TextInputInnerProps> = ({ lapisField, pl

return (
<>
<input
type='text'
class='input input-bordered w-full'
placeholder={placeholderText ?? lapisField}
onInput={onInput}
ref={inputRef}
list={lapisField}
value={initialValue}
/>
<div className='relative w-full'>
<input
type='text'
className='input input-bordered w-full pr-10'
placeholder={placeholderText ?? lapisField}
onInput={onInput}
ref={inputRef}
list={lapisField}
value={inputValue}
/>
{hasInput && (
<button
type='button'
name='✕'
className='absolute top-1/2 right-2 transform -translate-y-1/2 text-gray-500 hover:text-gray-700'
onClick={() => {
if (inputRef.current) {
inputRef.current.value = '';
onInput();
}
}}
>
✕
</button>
)}
</div>
<datalist id={lapisField}>
{data.map((item) => (
<option value={item} key={item} />
Expand Down
Loading