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

[First - PlatformSpecific] NavigationBar for Android #1678

Merged
merged 11 commits into from
Feb 15, 2024

Conversation

pictos
Copy link
Member

@pictos pictos commented Feb 5, 2024

Description of Change

This PR implements our first platform-specific feature. It's a move on from the approach taken on #1665.

Documentation will be created after the PR review.

Linked Issues

PR Checklist

Additional information

Screenbits.2024-02-05_013419.mp4

@pictos pictos requested a review from a team February 5, 2024 04:52
Copy link
Member Author

@pictos pictos left a comment

Choose a reason for hiding this comment

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

A couple of comments that could be helpful for anyone who wants to implement some Platform-Specific in the future.


internal static partial void RemapForControls()
{
PageHandler.Mapper.Add(ColorProperty.PropertyName, MapNavigationColorProperty);
Copy link
Member Author

Choose a reason for hiding this comment

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

Think this as the PropertyChanged for the BindableProperty, and this will be attached to the Page, but keep in mind that should be attached to the control that we'll implement the feature


namespace CommunityToolkit.Maui.PlatformConfiguration.AndroidSpecific;

static partial class NavigationBar
Copy link
Member Author

Choose a reason for hiding this comment

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

As you will see I created this as partial class, one to hold the properties that should be visible for all platforms, and the implementation, which can be visible just for the platform-specific, in this case android.

/// <summary>
/// Provides platform-specific configuration properties for the Android navigation bar.
/// </summary>
public static partial class NavigationBar
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the property class, where we hold everything that should be visible for all platforms

Comment on lines +105 to +112
internal static partial void RemapForControls();

#if !ANDROID
internal static partial void RemapForControls()
{

}
#endif
Copy link
Member Author

Choose a reason for hiding this comment

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

use this pattern here, so when we call this method on AppBuilderExtensions we don't need to use the #if <Platform>, so we do the mess just in one place.

Co-authored-by: Vladislav Antonyuk <33021114+VladislavAntonyuk@users.noreply.github.com>
@jfversluis jfversluis merged commit f4f8033 into main Feb 15, 2024
8 checks passed
@jfversluis jfversluis deleted the pj/navigationbar-platform-specific branch February 15, 2024 12:54
@jfversluis
Copy link
Member

Thank you for another great piece of work @pictos!

@jfversluis
Copy link
Member

Oops guess we still needed the documentation of course

@bijington
Copy link
Contributor

Oops guess we still needed the documentation of course

I'll jump on that tonight

@jfversluis
Copy link
Member

Thank you! Let me know if you can do it, else I will!

@bijington
Copy link
Contributor

@jfversluis you should find a docs PR waiting for your review :)

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.

[Proposal] Android-Specific Navigation Bar Effect
6 participants