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

[Feedback] New input focus behaviour added in v7.0.0 #1439

Closed
alex-mcgovern opened this issue Nov 21, 2022 · 5 comments · Fixed by #1440
Closed

[Feedback] New input focus behaviour added in v7.0.0 #1439

alex-mcgovern opened this issue Nov 21, 2022 · 5 comments · Fixed by #1440
Labels

Comments

@alex-mcgovern
Copy link

alex-mcgovern commented Nov 21, 2022

  • downshift version: 7.0.1
  • node version: v18.12.1
  • npm (or yarn) version: 8.19.2

This issue relates to new Downshift behaviour of opening menu on input focus, added in v7.0.0

  // /src/hooks/useCombobox/index.js#L446
  onFocus: callAllEventHandlers(onFocus, inputHandleFocus),

permalink


  • For my use case, I also wanted to attach toggleMenu from useCombobox to input's onClick handler, that way the user can toggle the menu after input has been focused should they wish
  • I experienced flickering of the menu on the initial click on the input, due to it both being focused, and receiving a click event
  • I expect this use case is somewhat common, and you may want to either
    a. reconsider this behaviour
    b. allow disabling this behaviour, perhaps as an option passed to getInputProps
    c. consider adding opening/toggling menu on input click as well as focus
import React from "react";
import { useCombobox } from "downshift";
import { Input } from "Input";

function DownshiftInput() {
  const {
    // ...
    getInputProps,
    toggleMenu,
    // ...
  } = useCombobox({
    // ...
  });

  return (
    <Input
      {...getInputProps({
        // ...
        onClick: toggleMenu, // <-- this now fights default input focus handler, causing flickering of the menu if not handled
      })}
    />
  );
}

For what it's worth, I am able to override this in stateReducer passed to useCombobox, but it took a few minutes to figure out, and isn't exactly intuitive to do.

  const {
    // ...
    getInputProps,
    toggleMenu,
    // ...
  } = useCombobox({
    // ...
    stateReducer(actionAndChanges) {
      const { changes, type } = actionAndChanges;

      switch (type) {
        case useCombobox.stateChangeTypes.InputFocus:
          return { ...changes, isOpen: false };

        default:
          return changes;
      }
    },
  });

Hope this feedback is useful.

@silviuaavram
Copy link
Collaborator

Hi, thanks for the feedback!

From the looks of it, InputFocus should become InputClick, and it should toggle. InputFocus should not do anything, actually, and can be removed.

Will keep the issue for tracking.

@silviuaavram silviuaavram added bug wip Work in progress labels Nov 22, 2022
@silviuaavram silviuaavram removed the wip Work in progress label Nov 23, 2022
@silviuaavram
Copy link
Collaborator

@alex-mcgovern you can check the PR attached and let me know what you think. I tried to come up with some sort of middle ground. I cannot do both focus and toggle click support since that will require additional hacks in useCombobox code. I'd rather not do that and instead just remove the focus behaviour without delivering a breaking change.

But let me know if you think there is a better way.

@alex-mcgovern
Copy link
Author

@silviuaavram Thanks for this, will check it out asap!

@alex-mcgovern
Copy link
Author

Hi @silviuaavram I've been moved onto something that doesn't use downshift, so can't really give this any attention — if you're happy with the changes, don't let me hold them up.

@silviuaavram
Copy link
Collaborator

Covered in v8

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

Successfully merging a pull request may close this issue.

2 participants