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

Add onMenuHide callback #210

Closed
chucksellick opened this issue Jul 20, 2017 · 3 comments
Closed

Add onMenuHide callback #210

chucksellick opened this issue Jul 20, 2017 · 3 comments

Comments

@chucksellick
Copy link

chucksellick commented Jul 20, 2017

Firstly thanks for this component, I looked at a few autocomplete components and they all seemed bloated and didn't really solve my fairly simple requirements, but this one was almost perfect right out of the box. However there is a small use case that it doesn't answer and I've worked around with a horrible hack for now ;) Hopefully it's a fairly simple feature request.

My use case is as follows:

  • I have an "+ Add year" button to tag a post with a year
  • Clicking this button causes the Typeahead to appear, populated with a list of the last 100 years. The input is autoFocused and the menu pops open immediately (great!)
  • When the user has finished with the control I destroy the control. If they didn't pick a year I show the button again, otherwise I show the chosen year in plain text (it is still clickable to change again).
  • I define "finished with the control" as any of:
    a) user chooses a year from the list (by clicking an item, or by typing / cursoring and pressing enter)
    b) user cancels by pressing esc
    c) control loses focus due to tabbing or by clicking elsewhere on the page

With the current API I'm unable to answer all three of these requirements. onBlur is not helpful here because destroying the Typeahead during the onBlur event means that (a) doesn't work - the input blurs and the component gets unmounted before the component can respond to the click on the menu item; so onChange never gets fired. My onBlur handler was simply this:

    onBlur = () => {
        this.setState({isOpen:false});
    }

And my render method (simplified for clarity) was like this:

    render() {
        return this.state.isOpen ?
            (
                <Typeahead
                    autoFocus
                    onChange={this.onChange}
                    onBlur={this.onBlur}
                    options={this.props.values()}
                    defaultSelected={[this.state.value]}
                />
            ) :
            (
                    <button onClick={this.openSelect} />
            );
    }

So, a tweak to onBlur to ensure that it happens after any menu clicks would partially solve this, but I still don't think it could solve (b). So all that is needed to fully solve my case would be a callback like "onCloseMenu" which gets fired any time the component closes the menu for any reason. Presumably it would be natural for this to happen after the onChange handler caused by clicking on the menu. Additionally for parity maybe there should be an onOpenMenu (but I don't have a use case for this).

Hopefully this sounds like a reasonable feature request and potentially not too hard to implement! My current hacky workaround is really a bit flaky and looks like this:

    onBlur = () => {
        // Removing the Typeahead from DOM too quickly means the onChange handler never gets called in the normal case
        // of a user clicking on a list item. Updating half a second later if the user tabs or clicks outside of the input
        // feels ok in operation but isn't ideal, and breaks if the user focuses back on the element too quickly.
        setTimeout(()=>this.setState({isOpen:false}), 500);
    }
@ericgio ericgio changed the title Need a callback for when the dropdown is closed Add onMenuHide callback Jul 20, 2017
@ericgio
Copy link
Owner

ericgio commented Jul 20, 2017

Thanks for the in-depth explanation and code examples. Seems pretty reasonable to add an onMenuHide or onMenuClose hook, as well as a corresponding show or open hook, and it wouldn't be too hard to do.

In the meantime, you might be able to just set the timeout to 0, which will allow the click to fire before the blur callback is called.

@ericgio ericgio added this to the 2.0 milestone Jul 20, 2017
ericgio added a commit that referenced this issue Jul 21, 2017
@ericgio
Copy link
Owner

ericgio commented Jul 21, 2017

Added in v2.0.0-alpha.1

@ericgio ericgio closed this as completed Jul 21, 2017
@chucksellick
Copy link
Author

No, a zero timeout didn't work - I tried this first of all, but something to do with React update cycle and/or DOM events apparently causes a significant delay between onBlur and onClick - even at 100ms delay it was intermittent (testing on a slowish laptop). 500ms was definitely enough so I didn't spend further time analysing but it's probably worth being aware of this, I'm guessing it would affect low spec computers more. Anyway thanks for the really fast update, much happier to have the proper solution ;)

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

2 participants