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

Component request - tree select #597

Open
tomaskikutis opened this issue Mar 21, 2022 · 14 comments
Open

Component request - tree select #597

tomaskikutis opened this issue Mar 21, 2022 · 14 comments

Comments

@tomaskikutis
Copy link
Member

tomaskikutis commented Mar 21, 2022

It needs to work in a generic way, but use cases I'm looking at right now are selecting subjects and authors.

Required features:

  • Selecting multiple items
  • Searching items (preferably in all levels, only applies to synchronous mode)
  • Ability to select entire branch even if it has children
  • The component has to accept props specified below in IProps

optionTemplate and valueTemplate are needed for custom rendering. For example, when selecting authors, avatars need to be rendered - optionTemplate would be used for this. valueTemplate would be used when listing selected values - see authors screenshot.

interface ITreeNode<T> {
    value: T;
    parent?: ITreeNode<T>;
    children?: Array<ITreeNode<T>>;
}

interface ITreeWithLookup<T> {
    nodes: Array<ITreeNode<T>>;
    lookup: {
        [id: string]: ITreeNode<T>;
    };
}

interface IPropsBase<T> {
    values: Array<T>;
    onChange(values: Array<T>): void;
    optionTemplate?: React.ComponentType<{item: T}>; // not required, it should fallback to rendering a label (from getLabel method)
    valueTemplate?: React.ComponentType<{item: T}>; // not required, it should fallback `optionTemplate` if not provided
    getId(item: T): string;
    getLabel(item: T): string;
    canSelectBranchWithChildren?: boolean;
    allowMultiple?: boolean;
    readOnly?: boolean;
}

interface IPropsSync<T> extends IPropsBase<T> {
    kind: 'synchronous';
    getOptions(): ITreeWithLookup<T>;
}

type ICancelFn = () => void;

interface IPropsAsync<T> extends IPropsBase<T> {
    kind: 'asynchronous';

    /**
     * The function will be called when a search is initiated from UI.
     * `callback` will be invoked with matching options after they are retrieved.
     * A function to cancel the asynchronous search is returned.
     */
    searchOptions(term: string, callback: (options: ITreeWithLookup<T>) => void): ICancelFn;
}

type IProps<T> = IPropsSync<T> | IPropsAsync<T>;

Note: see if you need to do lookups in the implementation. If not, use Array<ITreeNode<T>> instead of ITreeWithLookup<T>

@tomaskikutis
Copy link
Member Author

I have modified the initial interface to support asynchronous mode that is required for selecting locations from a remote geonames API.

2022-03-24_10-47

@tomaskikutis
Copy link
Member Author

I've added another property to the interface - allowMultiple. When it's not true, it should only allow one value and display a appropriate UI component, something like this:
2022-03-24_12-44

@dzonidoo
Copy link
Collaborator

#614

@tomaskikutis
Copy link
Member Author

tomaskikutis commented May 11, 2022

The interface doesn't match the one requested in #614

  • there's no asynchronous mode
  • templates do not allow to style individual options in the dropdown
  • canSelectBranchWithChildren doesn't receive a branch as an argument

@tomaskikutis tomaskikutis reopened this May 11, 2022
@dzonidoo
Copy link
Collaborator

@tomaskikutis
Copy link
Member Author

  • the interface doesn't conform to the one provided in this ticket (I don't mind additional fields)
  • the component is not exported from ui-framework, thus can't be imported from superdesk client

@tomaskikutis
Copy link
Member Author

@dzonidoo I tried testing it, but as mentioned in the previous comment, tree select component isn't exported from ui-framework and thus can't be used. It should be exported via app-typescript/index.ts.

Add "testing" label again when this is fixed.

@tomaskikutis
Copy link
Member Author

tomaskikutis commented Jul 29, 2022

It looks better @dzonidoo, but there are still issues:

import React from 'react';
import {TreeSelect} from 'superdesk-ui-framework/react';
import {ITreeNode} from 'superdesk-ui-framework/react/components/TreeSelect';

type IProps = {};

interface IVocabularyItem {
    qcode: string;
    name: string;
}

interface IState {
    value: Array<IVocabularyItem>;
}

const source = [
    {
        'name': 'Article (news)',
        'qcode': 'Article',
    },
    {
        'name': 'Sidebar',
        'qcode': 'Sidebar',
    },
    {
        'name': 'Factbox',
        'qcode': 'Factbox',
    },
];

function searchOptions(
    term: string,
    callback: (res: Array<ITreeNode<{name: string; qcode: string;}>>) => void,
): void {
    setTimeout(() => {
        callback(
            source
                .filter((item) => item.name.toLocaleLowerCase().includes(term.toLocaleLowerCase()))
                .map((item) => ({value: item})),
        );
    }, 1000);
}


export class MultiSelectDemo extends React.PureComponent<IProps, IState> {
    constructor(props: IProps) {
        super(props);

        this.state = {
            value: [],
        };
    }

    render() {
        return (
            <TreeSelect
                kind="asynchronous"
                searchOptions={searchOptions}
                value={this.state.value}
                onChange={(val) => {
                    this.setState({value: val});
                }}
                getLabel={({name}) => name}
                getId={({qcode}) => qcode}
                selectBranchWithChildren={false}
                optionTemplate={(item) => <span style={{color: 'blue'}}>{item.name}</span>}
                allowMultiple={true}
            />
        );
    }
}

@tomaskikutis
Copy link
Member Author

tomaskikutis commented Jul 29, 2022

✔️ One more thing, could you add a prop that would limit search only to a level that is being shown at any time? It's needed in authors select, because showing "editor" role doesn't make sense when it's not clear to which user does it belong.

@tomaskikutis
Copy link
Member Author

tomaskikutis commented Aug 23, 2022

@dzonidoo here are the issues I found after latest testing:

  • menu should open to the top if there's not enough space at the bottom (was already reported before) (partially fixed, causes flickering in async mode as described in following comment)
  • async mode - if search field is empty, searchOptions must not be called
  • fix flickering - after I finish typing, dropdown results change multiple times - it should only change once
  • "Search this category" and "Search" placeholders are not translatable (maybe we don't need them at all?)
  • nice-to-have: add support for keyboard navigation - visual focus indication, close on ESC, support navigation using arrows

@tomaskikutis
Copy link
Member Author

Results from latest testing on #669 at commit 8946db4

menu should open to the top if there's not enough space at the bottom (was already reported before)

This doesn't work well in async mode - video

fix flickering - after I finish typing, dropdown results change multiple times - it should only change once

doesn't look fixed - video


Newly found issues:

  • Race conditions are present in async mode. Example:
    - search field value set to a
    - network request initiated for a
    - search field value set to b
    - network request initiated for a
    - network request arrives for b
    - network request arrives for a

ACTUAL: search results for a are displayed
EXPECTED: search results for b are displayed

@tomaskikutis
Copy link
Member Author

Newly found issues:

  • read-only and disabled modes do not work in single select mode (items can still be selected)

@tomaskikutis
Copy link
Member Author

Newly found issues:

  • Causes rundown item editing to crash

To reproduce:

@tomaskikutis
Copy link
Member Author

@dzonidoo I didn't do much testing yet, but one of the issues I noticed when using it is that when readOnly is true, appropriate styling isn't being applied like for other fields. image

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

No branches or pull requests

3 participants