ListSelectController::change_index should Eq instead of Data::same()

[maurerdietmar]

Just saw that ListSelectController::change_index() use Data::same() to compare values:

    fn change_index(&self, data: &mut T, next_else_previous: bool) {
        if let Some(mut index) = self.variants.iter().position(|variant| variant.same(data)) { 

IMHO this should use Eq to compare variants. This muse be exact - not best effort.

[richard-uk1]

So Data::same implies PartialEq::eq, ie. it is stronger. The trade-off is that it takes less time, but is allowed to have false falses. It is not allowed to have false trues.

[maurerdietmar]

So Data::same implies PartialEq::eq, ie. it is stronger. The trade-off is that it takes less time, but is allowed to have false falses.

Exactly this is the problem. See Druid Radio for a more correct implementation...

[richard-uk1]

Ahh I see.

[maurerdietmar]

Besides, do you know the difference between ListSelect and RadioGroup? I can see that items are rendered differently, but other functionality is quite the same? I just wonder if we can merge that into RadioGroup...

[maurerdietmar]

I just wonder if we can merge that into RadioGroup...

Just detected that Flutter also use 2 different widgets, so I guess people expect to have different widget for different visual style.

[richard-uk1]

Interesting though. Perhaps they could share the logic code (I don't know just thinking out loud).

[richard-uk1]

The original observation is definitely a bug though.