Dropdown: add forceSelection prop

[darewreck54]

I'm not sure if this was by design, so I'm posted this question. Sorry, if i posted this in the wrong section.

Steps

1. https://codepen.io/anon/pen/eeBYKy?editors=1111
2. Notice by default, you see the placeholder for the dropdown
3. click on the drop down and don't select any item. Click away from the drop down. This will auto populated the first item in the drop down list. OnChange Event that is triggered, it doesn't give you any context if a specific item in the list is selected vs. a user clicking anywhere on the screen

Expected Result

1. User clicks away and does not select an item in the drop down. Expect the placeholder to still be displayed.

Actual Result

1. Always selects the first item in the dropdown list

Version

0.76.0

Testcase

https://codepen.io/anon/pen/eeBYKy?editors=1111

If this is by design, is there anyway around this? Goal: if user accidently clicks away and not on a list item, I want nothing to be selected. The way i determine if something is selected is onChange and examining the data.value field. This value is always set to the first element on the list even if you don't select the item.

Thanks,
Derek

[layershifter]

SUI has an options for this, it's called forceSelection and defaults to true. We should also impliment it. Contributions are welcome.

[darewreck54]

@layershifter I'll take a stab at it. I think i have the solution, but I'm not sure how to debug the unit test. I see that some of the test are failing. How do you debug the unit test? Currently i'm using vscode, but not sure how to configure or how the test infrastructure is setup for the project.

[darewreck54]

@layershifter Here is the PR i created. #2308

I still need to add test and verify they all work (having a little trouble getting started with the test infastructure). Please take a look and let me if I took the right approach.

[jjjkkklll]

I would like to add, that when clicking outside of the DD, onChange is triggered. But there is no change happening!? So I think it shouldn't be triggered? Or if it is triggered, then we need a notification if clicked outside or on an item.

[darewreck54]

If you click outside and there is an action of onchange, the value itself is unchange so the user can treat it as a noop. But yeah, unless there is a reason for a user to need this action then it should prob be removed. Not really sure where else it is being used.

…

Sent from my iPhone On Nov 15, 2017, at 5:05 AM, jjjkkklll <notifications@github.com<mailto:notifications@github.com>> wrote: I would like to add, that when clicking outside of the DD, onChange is triggered. But there is no change happening!? So I think it shouldn't be triggered? Or if it is triggered, then we need a notification if clicked outside or on an item. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#2299 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/APbCznrU6gkszOlGHH6YffdHS3rhwV57ks5s2uGrgaJpZM4QVqrr>.

[jjjkkklll]

I agree, but still, this feels like a workaround. Because the word "onChange" suggests that it is triggered by an actual change of the DD. This is absolutely not critical, but still maybe worth a look while reviewing this part of the code. On the other hand, I am not sure if there is a trigger for clicking outside, which would have to be implemented then. Changing the docs is probably the more easy way.

[darewreck54]

@layershifter can onChange be removed from being triggered? Wil lthat break anything else?

[darewreck54]

@jjjkkklll @layershifter check out https://github.com/darewreck54/Semantic-UI-React/blob/07009b052888c7b854faef6aebf3cc49a50a03a5/src/modules/Dropdown/Dropdown.js#L589

You will notice that the code trigger the setSelectetIndex and right after calls handleChange. handleChange only takes in the new value. There is a comment that you cannot rely on props.value because its controlled component. So no real comparison is made. I'm guessing the the caller has to be responsible to determine if handleChange needs to be made or not based on diffing the old vs. new value. You could also put the logic into handleChange, but then you would have to pass in a new and old value. Not sure what controller component refers to and how props value would not be accurate.

[layershifter]

I've tested proposed changes in #2308. BTW, @darewreck54 thanks for your activity in this question.

My idea that we don't need to touch something in current bevahiour of onChange, especially in #2308. There are numerous problems and RPCs that affects Dropdown, we will solve them systematically. After solving issues with refs and transitions, I will do serious refactoring there, 1.4K LOC make Dropdown very compilicated and untestable.

[levithomason]

Per the expected result:

User clicks away and does not select an item in the drop down. Expect the placeholder to still be displayed.

We have the selectOnBlur prop, which when set to false will not select the item when the user blurs (clicks away) from the Dropdown.

https://codepen.io/levithomason/pen/MOBKpj

Sadly, it looks like we're missing an example for this in the docs. PRs welcome there, too.

I'll close this since that prop solves the OP issue. Will reopen if I've misunderstood or overlooked another aspect of this.

[darewreck54]

@levithomason Interesting.....does that mean that the added property forceSelection I added in the PR does the same thing or that it was just a side effect of selectOnBlur feature. Or is selectOnBlur and forceSelection the same thing?

[levithomason]

@darewreck54 I believe they are accomplishing the same thing. Seems it was reported in #2308 (comment) to also work for the use case.

LMK if your use case isn't handled by selectOnBlur and we can reopen this.

[ghost]

For me selectOnBlur is the way.