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.

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

Conversation

anna-parker
Copy link
Collaborator

@anna-parker anna-parker commented Jan 2, 2025

resolves #514

Summary

Currently deleting the input of the lineage or text field is tedious as it requires selecting the entire input and hitting deleting, this PR adds a simple x to delete input in one go. It also sorts the input of the text-input field.

Screenshot

Screen.Recording.2025-01-02.at.16.56.01.mov

PR Checklist

  • All necessary documentation has been adapted.
  • The implemented feature is covered by an appropriate test.

Copy link

vercel bot commented Jan 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dashboard-components ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 2, 2025 3:56pm

Copy link
Contributor

github-actions bot commented Jan 2, 2025

This is a preview of the changelog of the next release. If this branch is not up-to-date with the current main branch, the changelog may not be accurate. Rebase your branch on the main branch to get the most accurate changelog.

Note that this might contain changes that are on main, but not yet released.

Changelog:

0.11.4 (2025-01-02)

Features

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

Bug Fixes

  • components: gs-aggregate: rerender when initialSortField or initialSortDirection changes (60d578f)

…e-filter input with x and sort text-input datalist options
Comment on lines +48 to +49
const [hasInput, setHasInput] = useState<boolean>(!!initialValue);
const [inputValue, setInputValue] = useState(initialValue || '');
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;

Comment on lines +108 to +113
onClick={() => {
if (inputRef.current) {
inputRef.current.value = '';
onInput();
}
}}
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)

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) => {

@@ -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.

@@ -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.

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

@@ -45,6 +45,9 @@ const LineageFilterInner: FunctionComponent<LineageFilterInnerProps> = ({

const inputRef = useRef<HTMLInputElement>(null);

const [hasInput, setHasInput] = useState<boolean>(!!initialValue);
const [inputValue, setInputValue] = useState(initialValue || '');
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 ?? '');

<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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow sorting of output fields in gs-aggregate
2 participants