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

check container content is null #3

Closed
wants to merge 3 commits into from
Closed

check container content is null #3

wants to merge 3 commits into from

Conversation

VladislavAntonyuk
Copy link

fix #2

@mariusmuntean
Copy link
Owner

Hey @VladislavAntonyuk ,

Thanks for your PR!
I had a look at it and I'm willing to merge it after you adjust it a bit.

First, it says it fixes #2 but actually it does a bit more, like extending the sample with SwipeViews. I didn't see any value in that; the sample should highlight what the coordinator layout can do.

Then I noticed that you updated Xamarin.Forms but I think the lib should reference an older version, thus allowing developers to integrate it into their projects without forcing them to also upgrade Xamarin.Forms. From my understanding the lib can declare a dependency on version x and be used in an app that uses version x and higher.

Looking forward for your contribution!

@VladislavAntonyuk
Copy link
Author

I thought issue #1 may be solved by simple update of Xamarin Forms. I will downgrade it.

As for the sample, it a test to display the issue #1. Looks like Input and CascadeInput do not work as expected. Swipe actions and even button clicks don’t work. Event doesn’t occur.

@VladislavAntonyuk
Copy link
Author

@mariusmuntean any updates?

@mariusmuntean
Copy link
Owner

I will have some time next weekend to have a closer look.
It is excellent of you to provide a fix for the NRE in this PR. I don't think it makes sense to also include updates to the sample which highlight another issue. I would prefer if you create the smallest possible project (doesn't have to be based on the sample app) that reproduces the issue and then link to it in the respective issue.

@VladislavAntonyuk VladislavAntonyuk deleted the vantonyuk-2-fix-nre branch August 31, 2020 20:20
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.

NRE if AutohideActionView is false by default
2 participants