Place text in searchbar

mnidhk commented 5 years ago

Nice code for implementing searchbar! Thnx for that. Did had some code before to do this but this one is very straightforward. Looking forward to some extended examples with Large title and tabpages for example One issue i have: When in code behind i set SearchText="??" it does update the SearchTextProperty but it isn't visible/changed on the Searchbar. Is this possible?

jtaubensee commented 5 years ago

Hi @mnidhk - Thanks for the issue. I just pushed a change which will respond to PropertyChanged events, and update the Text in the UISearchBar when the text is set on the iOSSearchPage. Let me know if it doesn't make sense.

d16080e03b6503a105bc8d493d214f887a6aebd8

mnidhk commented 5 years ago

Hi @jtaubensee - Thank you for your fast response! Your change works perfect! Manny thanks!

mnidhk commented 5 years ago

Hi @jtaubensee, After implementing the code in my app, i did found another issue. I used the code on a page which is used in a Master-Detail structure. When i switch from the page with searchbar to another one (from master menu) and going back (the page is still in memory because it is a detail page ) the function "OnElementChanged" is fired again. So this code is fired also : iosSearchPage.PropertyChanged += this.OnSearchPagePropertyChanged;

The code "oldSearchPage.PropertyChanged -= this.OnSearchPagePropertyChanged;" doesn't work/not fired and so the "iosSearchPage.PropertyChanged" has now 2 bindings to "OnSearchPagePropertyChanged"

I needed to change the function "OnSearchPagePropertyChanged": if (this.searchController.SearchBar != null) { this.searchController.SearchBar.Text = iOSSearchPage.SearchText; }

This function is now fired 2 times! The first time the searchbar is NULL so it crashed without my check. The second time it is fired, it works. When i switch again from pages in my menu and go back to this page, it get another binding. So the function is fired 3 times, the first 2 times with searchbar is Null and the 3rd time it works. Hope i made myself clear. So is there a possibility to clear the "iosSearchPage.PropertyChanged" bindings?

jtaubensee commented 5 years ago

HI @mnidhk - Do you by chance have a repro you can share? I use this search page within a Tabbed page, which I would expect to behave the same as a Master Detail page.

In the meantime, do you know if this is getting hit?

https://github.com/jtaubensee/SearchApp/blob/d16080e03b6503a105bc8d493d214f887a6aebd8/SearchApp.iOS/Components/Renderers/SearchPageRenderer.cs#L99-L102

Or in other words, are the NewElement and the OldElement populated and not null?

mnidhk commented 5 years ago

Hi @jtaubensee , i will try to create an small test project in the next few days.

No, in my app these lines are never being hit. When i switch from pages the OldElement is always NULL and NewElement is always populated. So that's why that the line iosSearchPage.PropertyChanged += this.OnSearchPagePropertyChanged; is always activated when coming back to this page. So the "Propertychanged" binding gets multiple bindings.

mnidhk commented 5 years ago

For now, here is a screen recording of what happens: https://we.tl/t-BMykHs65X8

jtaubensee commented 5 years ago

@mnidhk - Any luck coming up with a repro?

mnidhk commented 5 years ago

@jtaubensee Sorry! I really want to but it isn't going to happen this week due many factors. Will try this weekend but else on Monday for sure!

mnidhk commented 5 years ago

@jtaubensee

I created a testproject from scratch. Created 2 folders: "Helpers" in .NET project and iOS with your code. -Start project and type something in search.

Open menu and go to other page "About"
Go back to 1st page and the searchbar is initializing again.
Type something again in search and "OnSearchPagePropertyChanged" is fired twice. One with "null" and one with the right object. Without my extra check it would crash. Switch again between the pages and there would be an another binding and the function is fired three times etc. The problem off course is iosSearchPage.PropertyChanged += this.OnSearchPagePropertyChanged; It does not clear the propertyChanged binding. Don't know if this is possible? Thanks in advance for trying to solve this!

https://we.tl/t-YWEes2EBgk

jtaubensee commented 5 years ago

@mnidhk - Adding the file you sent here for reference: SearchTest.zip

I think I found a solution. Since e.OldElement is always null, we need to unsubscribe the PropertyChanged event another way. We can actually add this code to the overridden Dispose method.

// Added on line 161
if (this.Element is ModernContentPage modernContentPage)
{
    modernContentPage.PropertyChanged -= this.OnSearchPagePropertyChanged;
}

Actually this is probably a better practice anyway. To my knowledge, the garbage collector would never collect the renderer since there was still an event subscribed. I will make sure to add this back to the sample.

Hope this helps, and thanks for bringing it to my attention!

mnidhk commented 5 years ago

@jtaubensee sorry for the late response! I did test your findings and now it triggers only once. So everything looks very good now! Thank you very much for your time and effort!

Ps. Don't forget to also change the lines in your example

jtaubensee / SearchApp

Place text in searchbar #1