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 for Actionbar at UpdateActionBar method. #366

Merged
merged 1 commit into from Oct 20, 2016

Conversation

Projects
None yet
4 participants
@gleblebedev
Contributor

gleblebedev commented Sep 19, 2016

Description of Change

ActionBar may not be present in certain cases. For instance there is no ActionBar in full screen applications.

Bugs Fixed

https://bugzilla.xamarin.com/show_bug.cgi?id=25677
https://forums.xamarin.com/discussion/55045/amazon-fire-tv-stick

API Changes

None.

Behavioral Changes

None except application now can work in a fullscreen mode.

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

The change works for me for a year now. As you can see from this code snippet
https://gist.github.com/gleblebedev/e89e1d77d76644142d44
I was modifying binary IL code to make the change because no source code was available. Since it's open source now I suggest to add it to repository.

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Sep 19, 2016

Hi @gleblebedev, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

dnfclas commented Sep 19, 2016

Hi @gleblebedev, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

@@ -427,6 +427,10 @@ internal static void SetPageContext(BindableObject bindable, Context context)
internal void UpdateActionBar()
{
if (ActionBar == null) //Fullscreen theme doesn't have action bar

This comment has been minimized.

@samhouts

samhouts Sep 20, 2016

Member

ActionBar is directly referenced in dozens of other places in this file without a null check. Does it not error anywhere else?

@samhouts

samhouts Sep 20, 2016

Member

ActionBar is directly referenced in dozens of other places in this file without a null check. Does it not error anywhere else?

This comment has been minimized.

@gleblebedev

gleblebedev Sep 22, 2016

Contributor

It does not. But I'm only concern about one scenario - when app is running in a full screen mode. And it works for me with the fix.

@gleblebedev

gleblebedev Sep 22, 2016

Contributor

It does not. But I'm only concern about one scenario - when app is running in a full screen mode. And it works for me with the fix.

@jassmith jassmith merged commit 40659f4 into xamarin:master Oct 20, 2016

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment