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

Suggest2 does not render text box values on page load and text input dissapears #5804

Closed
moon1ock opened this issue Dec 15, 2022 · 5 comments · Fixed by #5806
Closed

Suggest2 does not render text box values on page load and text input dissapears #5804

moon1ock opened this issue Dec 15, 2022 · 5 comments · Fixed by #5806

Comments

@moon1ock
Copy link
Member

moon1ock commented Dec 15, 2022

This can be seen on the Blueprint docs

Screen.Recording.2022-12-15.at.13.32.59.mov
@moon1ock
Copy link
Member Author

Even though it might look like expected behavior, I am not sure if it is (am open to discussion)

The rationale is based on this code:

@moon1ock
Copy link
Member Author

moon1ock commented Dec 15, 2022

in packages/select/src/components/suggest/suggest2.tsx

    private getPopoverTargetRenderer =
        (listProps: QueryListRendererProps<T>, isOpen: boolean) =>
        // eslint-disable-next-line react/display-name
        ({
            // pull out `isOpen` so that it's not forwarded to the DOM
            isOpen: _isOpen,
            // pull out `defaultValue` due to type incompatibility with InputGroup
            defaultValue,
            ref,
            ...targetProps
        }: Popover2TargetProps & React.HTMLProps<HTMLInputElement>) => {
            const { disabled, fill, inputProps = {}, inputValueRenderer, popoverProps = {}, resetOnClose } = this.props;
            const { selectedItem } = this.state;
            const { handleKeyDown, handleKeyUp } = listProps;

            const selectedItemText = selectedItem == null ? "" : inputValueRenderer(selectedItem);
            const { autoComplete = "off", placeholder = "Search..." } = inputProps;
            // placeholder shows selected item while open.
            const inputPlaceholder = isOpen && selectedItemText ? selectedItemText : placeholder;
            // value shows query when open, and query remains when closed if nothing is selected.
            // if resetOnClose is enabled, then hide query when not open. (see handlePopoverOpening)
            const inputValue = isOpen ? listProps.query : selectedItemText ?? (resetOnClose ? "" : listProps.query);

Consider that
const selectedItemText = selectedItem == null ? "" : inputValueRenderer(selectedItem); always replaces a null in selectedItemText to an empty string,

However the nullish coalescing here
const inputValue = isOpen ? listProps.query : selectedItemText ?? (resetOnClose ? "" : listProps.query);
checks whether selectedItemText is null and if it is -- renders props.query

selectedItemText is never null and "" is not equal to null

resultantly, the field always evaluates to empty string, even when it's supposed to render selectedItemText.

Due to this, our front end does not pre-populate these text boxes on page load. (This was discussed on slack and ghe, will not link here).

@moon1ock moon1ock assigned adidahiya and moon1ock and unassigned adidahiya Dec 15, 2022
moon1ock added a commit that referenced this issue Dec 15, 2022
@moon1ock
Copy link
Member Author

@ted-jenks for SA and collaboration

@adidahiya
Copy link
Contributor

Good catch; this certainly looks like a regression from Suggest. I don't have a v4 documentation link with a Suggest demo but you can see it working correctly in v3: https://blueprintjs.com/docs/versions/3/#select/suggest

The regression is due to a bad port of this line from Suggest to Suggest2 (?? behaves differently from ||):

: selectedItemText || (this.props.resetOnClose ? "" : listProps.query);

const inputValue = isOpen ? listProps.query : selectedItemText ?? (resetOnClose ? "" : listProps.query);

@moon1ock
Copy link
Member Author

moon1ock commented Jan 4, 2023

Thank you @adidahiya @ted-jenks !!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants