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

Fixes #2140. OnSelectedChange is being called from OnEnter when the selection didn't actually change. #2152

Merged
merged 5 commits into from Nov 2, 2022

Conversation

tig
Copy link
Collaborator

@tig tig commented Nov 1, 2022

Fixes #2140.

OnSelectedChange is being called from OnEnter when the selection didn't actually change.

The fix broke ComboBox - this test: HideDropdownListOnClick_True_Highlight_Current_Item ()

image

System.Exception : Colors used did not match expected at row 1 and col 0 (indexes start at 0). Color index used was Black,Black but test expected Black,Black (these are indexes into the expectedColors array)

Which is confusing to me...

@tig
Copy link
Collaborator Author

tig commented Nov 1, 2022

@BDisp - since you are familiar with ComboBox, can you please look at this PR and figure out how to adjust ComboBox so that it is no longer dependent on ListView.OnSelectedChanged being called from ListView.OnEnter?

And submit a PR to this PR?

@BDisp
Copy link
Collaborator

BDisp commented Nov 1, 2022

Ok @tig .

@tznind
Copy link
Collaborator

tznind commented Nov 1, 2022

Black,Black but test expected Black,Black

Black was not black enough! :goth:

Maybe we should change this test to do 'by value' rather than 'by reference' comparison on those. Unless it is that the error message is bugged itself.

That said it could just be an issue with the test itself:
image

@BDisp
Copy link
Collaborator

BDisp commented Nov 1, 2022

I can't see it right now, but it seems that the background is cleared after it was redraw.

@BDisp
Copy link
Collaborator

BDisp commented Nov 1, 2022

The extra 0 is the down arrow. I already realized that the errors messages are bugged. The only valid information I get is the row, col and if the color already exists in the variable. But I think that it should inform what the color is in the test and what color it should were be.

@BDisp
Copy link
Collaborator

BDisp commented Nov 1, 2022

@tig the fix is here #2156 (comment).

@BDisp
Copy link
Collaborator

BDisp commented Nov 2, 2022

@tig I submitted a PR in your branch tig#4.

@tig tig marked this pull request as ready for review November 2, 2022 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ListView: OnSelectedChanged is called when it didn't actually change (OnEnter)
3 participants