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

feat(Android): Add support for different Top App Bar styles #1874

Open
wants to merge 56 commits into
base: main
Choose a base branch
from

Conversation

tboba
Copy link
Member

@tboba tboba commented Aug 21, 2023

Description

Since the beginning of time...
Since the first version of react-native-screens...
There was a toolbar.

But toolbar have always been...
Small and non-expandable...

Nowadays...
times have changed.

This Pull Request introduces not 3, not 3.5, but 4 types of headers - center-aligned, small, medium and large header types, where the last two of them can collapse.
From now the toolbar will always be nested in CollapsingToolbarLayout, but this shouldn't have an impact on the experience overall.
Unfortunately, while having header type set to medium or large the headerCenter property does not work for now.

For the best experience of using new header types it is recommended to have Material 3 bundled into the application, while using new header types.

For more information about the header types, please visit official Material 3 documentation.

Changes

  • Added support for new header types
  • Changed the way how hierarchy is being created during the ScreenStackFragment inflation
  • Changed the behaviour of positioning items on header (via headerLeft and headerRight prop)
  • Added Material 3 to our example apps

Screenshots / GIFs

Before

Screen.Recording.2023-09-01.at.17.22.13.mov

After

8mb.video-WMw-Zz1dZmWl.mp4

Test code and steps to reproduce

Use Test1874 attached to this branch to test the changes in TestsExample & FabricTestExample.

Checklist

@tboba tboba added the Platform: Android This issue is specific to Android label Aug 21, 2023
@tboba tboba self-assigned this Aug 21, 2023
@tboba tboba force-pushed the @tboba/introduce-material3-finally-prettier-android-ui branch from b4b31b9 to cf32cbc Compare August 24, 2023 10:11
@tboba tboba removed the Platform: Android This issue is specific to Android label Nov 30, 2023
@tboba tboba requested review from WoLewicki and kkafar and removed request for WoLewicki, kacperkapusciak and kkafar January 8, 2024 18:14
Copy link
Member

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

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

Hey, I added a review. Let's make sure not to introduce bugs with this 🚀

@@ -1,7 +1,7 @@
package com.swmansion.rnscreens

import android.content.Context
import androidx.appcompat.widget.Toolbar
import com.google.android.material.appbar.MaterialToolbar
Copy link
Member

Choose a reason for hiding this comment

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

We are sure it is safe to use on all configurations we support yes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I just ran the app on API 28 (with Material 3 and Material 2) and I can't observe any abnormalities, it works as usual 👍

// On Material 3 the elevation is not visible on AppBarLayout.
// To prevent this behavior, we're setting outline shadow colors to black.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
outlineAmbientShadowColor = Color.BLACK
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it is something that could be controlled by the user?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean the color of the elevation? I believe I've read somewhere the suggestion to add such modifier, so I believe we can do such thing! But let's maybe leave this idea for another PR.

Copy link
Member

Choose a reason for hiding this comment

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

Great! Keep it in mind.

// Reattach the header, when the developer tries to change the header type
if (screenStackHeader?.loadedHeaderType != screen?.headerType) {
screenStackHeader?.updateHeaderType()
screenFragment?.removeToolbar()
Copy link
Member

Choose a reason for hiding this comment

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

Why do we remove it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's because of updating the layoutParams of appBarLayout. Basically, when we're changing the type of appBar, its layout is being changed, resulting in wrong position of the views. For that reason we're removing all of its views (they will be added further in onUpdate) and setting references of toolbar and collapsingtoolbarlayout to null to ensure that we won't use old toolbars

src/native-stack/types.tsx Outdated Show resolved Hide resolved
native-stack/README.md Outdated Show resolved Hide resolved
@sidorchukandrew
Copy link

This is very exciting! Awesome work!

@tboba
Copy link
Member Author

tboba commented Apr 16, 2024

Currently, this PR is blocked by changes from facebook/react-native#44099 (we may skip requiring setting the prop nestedScrollEnabled with this change, but we'll see).

@sidorchukandrew
Copy link

That could make sense! We already set contentInsetAdjustmentBehavior on ScrollView for the large iOS headers.

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.

None yet

3 participants