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

Add a TextChanged Event to the EntryCell #227

Merged
merged 14 commits into from
Nov 19, 2018

Conversation

londospark
Copy link

@londospark londospark commented Nov 13, 2018

Started work on the TextChanged event to start fixing #225 - it seems to work in WPF but not in iOS/Android. The Text property seems to be ignored in iOS/Android and needs looking into.

Please don't merge this yet as it does not work!

@TimLariviere
Copy link
Member

PR #226 needs to be merge before this one.
Also a rebase will be needed once done.

@TimLariviere
Copy link
Member

Great job, especially while doing that on stream! :)

@TimLariviere
Copy link
Member

One last thing :)
You need to add a reference to Fabulous.CustomControls.dll in the template projects and samples projects as well, like you did for AllControls

@TimLariviere
Copy link
Member

I think I know why this doesn't work on Android and iOS.
You're hiding the base property Text of EntryCell with the one you declared in CustomEntryCell.
Android and iOS still refer to EntryCell.Text, and without override they won't call your code.

For some reason, WPF is OK with that though.

@londospark
Copy link
Author

Thank you - I'll have a look at this tonight as well as cleaning up the PR.

@londospark
Copy link
Author

londospark commented Nov 15, 2018

I think I know why this doesn't work on Android and iOS.
You're hiding the base property Text of EntryCell with the one you declared in CustomEntryCell.
Android and iOS still refer to EntryCell.Text, and without override they won't call your code.

For some reason, WPF is OK with that though.

I've just had a look at Xamarin.Forms and the base Text property isn't virtual, so while we can hide it, we can't simply override it. I wonder if I need to have a closer look at trying to do this in Xamarin.Forms directly and just bubble up the change once it's in there.

@TimLariviere
Copy link
Member

Another way you could do it is by subscribing to the PropertyChanged event of EntryCell yourself.
And if the property changed is Text, then you can trigger your TextChanged event.

@londospark
Copy link
Author

Why did I not think of that? Thank you!!!

@londospark
Copy link
Author

This now works across all three platforms. If there's anything I can do to make any merge easier please let me know - I still haven't really got the hang of PRs, Git nor working in open source yet so apologies if and when I go wrong.

@TimLariviere
Copy link
Member

TimLariviere commented Nov 16, 2018

@TimLariviere
Copy link
Member

And finally, it would be great if you could move Fabulous.CustomControls into the src folder.

No need to exclude Fabulous.CustomControls from the Build step. This step will run dotnet pack on all src/**/*.fsproj, which we'll need to push the new project in NuGet.
You can just change your BuildCustomControls to do a MSBuild Release instead (only generate the dll for the Generator). Make sure you have the correct path to the dll in the bindings json afterwards.

@TimLariviere
Copy link
Member

This looks good. Well done!
I'll do a little preparation work and then will be able to release this in the next version :)

@TimLariviere TimLariviere merged commit 800c1f7 into fabulous-dev:master Nov 19, 2018
@TimLariviere TimLariviere mentioned this pull request Nov 19, 2018
@londospark londospark deleted the 225-EntryCell branch November 20, 2018 18:03
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.

2 participants