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

Space trigger dropdown visibility on inputs. Perhaps an option to disable openOnSpace #3768

Closed
gencer opened this issue Aug 31, 2019 · 9 comments

Comments

@gencer
Copy link

gencer commented Aug 31, 2019

Bug Report

Steps

  1. Create a dropdown
  2. Give trigger an input field. Add some menu items to dropdown.
  3. Click on input, you will see dropdown menu with items
  4. Now write something to input. It's all ok.
  5. Now try to press spacebar inside the input. Instead of adding a space to the text, it close or open dropdown.

Expected Result

I should be able to use space in input and not trigger menu visibility

At least, we need a prop for this specific usage.

See related issue here: #3146

See related code here:

openOnSpace = (e) => {
debug('openOnSpace()')
if (keyboardKey.getCode(e) !== keyboardKey.Spacebar) return
e.preventDefault()

To fix here, I commented e.preventDefault(); as // e.preventDefault();.

In fact, i can't even press space when dropdown menu is visible. Needs to be closed first. To be able to fix that, I had to remove spacebar keycode check condition from here:

keyboardKey.getCode(e) !== keyboardKey.Enter &&
keyboardKey.getCode(e) !== keyboardKey.Spacebar
)

Also to achieve expected result; I had to set closeOnChange to false.

For now, I monkey-patched via patch-package.

Actual Result

dropdown close and open on each spacebar press

Version

0.88.0

@gencer gencer changed the title Space trigger dropdown visibilty on inputs. Perhaps an option to disable openOnSpace Space trigger dropdown visibility on inputs. Perhaps an option to disable openOnSpace Aug 31, 2019
@layershifter
Copy link
Member

Thank for reporting. It's a bug 😿

Duplicate of #3764, will be fixed by #3766 and released during this week.

@gencer
Copy link
Author

gencer commented Sep 2, 2019

@layershifter, you underatand wrong (or i got it wrong).

The issue there is about when search prop is true. Meaning, dropdown has search bar on top of it. This is fixed, yes. But my use case is different. I gave input field as a trigger. I do not have search prop.

For example;

<Dropdown trigger={<Input onChange={this.onTextChange} />} fluid>

   ... menu items ...

</Dropdown>

Now when I press spacekey on <Input /> which is a trigger of dropdown, it opens/close dropdown menu. So, I think we explicitly need closeOnSpace prop here.

If I put search, does SemanticUI adds any search bar? Or is it just a common prop for such case? In this case, I would have 2 inputs. One is from tirgger, one from dropdown menu. I cannot test this because its not released yet.

@layershifter
Copy link
Member

Please provide a minimal repro case, it will be easier to understand the issue.

@gencer
Copy link
Author

gencer commented Sep 2, 2019

Sure. You're right actually.

It's on the way. Once finished, gonna publish here

@gencer
Copy link
Author

gencer commented Sep 2, 2019

@layershifter here you go:

https://codesandbox.io/s/semantic-ui-react-e0cj6

Try to write words separated by space. Instead of adding space to input, it will either close or open dropdown and no space will be inserted.

As I said, I use input as a trigger. You can see that in code.

@layershifter
Copy link
Member

layershifter commented Sep 2, 2019

Thanks 👍 It's the same bug as in #3764.

0.87.3: https://codesandbox.io/s/semantic-ui-react-toyjb
0.88.0: https://codesandbox.io/s/semantic-ui-react-wnu42


However, the fix that was introduced in #3766 will not fix this issue. Okay, the next questions.

Why you need to have Input as trigger and Children API at all?
Are there reasons why you can't use selection & search: https://react.semantic-ui.com/modules/dropdown/#types-search-selection?

@gencer
Copy link
Author

gencer commented Sep 2, 2019

I do not filter things. The search feature in Dropdown filters items by search value. My intention is completely different. I do not want to filter things, i do not want to select based on value. My intention is to provide suggestions on my own.

Live action:

4

As you can see i can put space as i already patched myself.

@layershifter
Copy link
Member

I do not filter things. The search feature in Dropdown filters items by search value.

You can do this by noop search function: https://codesandbox.io/s/semantic-ui-example-6213c
This option is also more accessible.


And now I see that your issue is closer to #3146. As now we improved focus handling, probably we can try to bind event listeners to the input field instead of document, it should fix the both issues.

@gencer
Copy link
Author

gencer commented Sep 2, 2019

I do not filter things. The search feature in Dropdown filters items by search value.

You can do this by noop search function: codesandbox.io/s/semantic-ui-example-6213c
This option is also more accessible.

Well, this is much much cleaner. I will use this way. (Instead of monkey-patching)

Thank you!

And now I see that your issue is closer to #3146. As now we improved focus handling, probably we can try to bind event listeners to the input field instead of document, it should fix the both issues.

Will be great in both way.

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