Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[UWP] connect the actual ObservableCollection to the ItemsSource #725

Merged
merged 6 commits into from Feb 23, 2017

Conversation

StephaneDelcroix
Copy link
Member

@StephaneDelcroix StephaneDelcroix commented Jan 27, 2017

Description of Change

[UWP] connect the actual ObservableCollection to the ComboBox.ItemsSource as it looks like ComboBox doesn't correctly connect to INPC or INPP on a non-ObservableCollection<> ItemsSource. We'd have to look at the ComboBox source to be 100% sure...

The workaround is then to expose our collection as internal

Bugs Fixed

API Changes

Behavioral Changes

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense

@pauldipietro
Copy link
Contributor

@StephaneDelcroix: On my end, it looks like that the changes break this on Android (both FormsActivity/AppCompat) and the items aren't showing up. iOS seems okay.

There's another issue I'm seeing too where if you add a label to the repro and have it set the text to whatever the SelectedItem is when SelectedIndexChanged fires, the first time something is selected, it's null. That appears to be occurring here because the event's getting invoked before the SelectedItem is updated; swapping their order appears to fix the issue (it also looks vaguely similar to 39407). If you want, I could make that change as another PR for history clarity, or you could include it in this one, since I'm assuming it's legitimate?

@StephaneDelcroix
Copy link
Member Author

@pauldipietro push to this branch, it's fine

@pauldipietro
Copy link
Contributor

I pushed that change which you can take a peek at, but Android still has issues with the items not showing up.

@TomQv
Copy link

TomQv commented Feb 21, 2017

When will this be merged?
Its desperatly awaited...

Copy link
Contributor

@pauldipietro pauldipietro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be okay with [Preserve] having been added in for Android.

@pauldipietro pauldipietro merged commit 81d6812 into master Feb 23, 2017
@StephaneDelcroix StephaneDelcroix deleted the fix-bz51642 branch March 3, 2017 11:58
@samhouts samhouts added this to the 2.3.5 milestone Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants