-
Notifications
You must be signed in to change notification settings - Fork 352
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
Replace third party top bar library #5231
Replace third party top bar library #5231
Conversation
DROID-272 Evaluate replacing collapsing toolbar
We should evaluate whether to replace the current collapsing toolbar since the project seem dead (https://github.com/onebone/compose-collapsing-toolbar). The following seems like a good potential: https://github.com/germainkevinbusiness/CollapsingTopBarCompose Update: There is now https://developer.android.com/jetpack/compose/components/app-bars#medium which we strongly should consider instead. |
2a53fa3
to
4427971
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 11 files at r1, 10 of 10 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
All approved |
0852662
to
9338a9e
Compare
e9f62da
to
a8b4bbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 11 files at r1, 3 of 10 files at r2, 1 of 2 files at r6, 1 of 2 files at r7, all commit messages.
Reviewable status: 15 of 17 files reviewed, 2 unresolved discussions (waiting on @Pururun and @Rawa)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt
line 88 at r7 (raw file):
} @OptIn(ExperimentalMaterial3Api::class)
Can this be removed when using @file:OptIn(ExperimentalMaterial3Api::class)
?
Code quote:
@OptIn(ExperimentalMaterial3Api::class)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt
line 192 at r7 (raw file):
} @OptIn(ExperimentalMaterial3Api::class)
Can this be removed when using @file:OptIn(ExperimentalMaterial3Api::class)
?
Code quote:
@OptIn(ExperimentalMaterial3Api::class)
a8b4bbf
to
95b2016
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 14 of 17 files reviewed, 2 unresolved discussions (waiting on @albin-mullvad and @Pururun)
android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt
line 88 at r7 (raw file):
Previously, albin-mullvad wrote…
Can this be removed when using
@file:OptIn(ExperimentalMaterial3Api::class)
?
Yeah, good catch!
android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/component/TopBar.kt
line 192 at r7 (raw file):
Previously, albin-mullvad wrote…
Can this be removed when using
@file:OptIn(ExperimentalMaterial3Api::class)
?
Done.
95b2016
to
eb11321
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r10.
Reviewable status: 12 of 17 files reviewed, 2 unresolved discussions (waiting on @albin-mullvad and @Pururun)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r11, all commit messages.
Reviewable status: 12 of 17 files reviewed, 2 unresolved discussions (waiting on @albin-mullvad)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 11 files at r1, 1 of 2 files at r7, 1 of 1 files at r8, 1 of 1 files at r10, 4 of 4 files at r11, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved
eb11321
to
7e28f78
Compare
Material 3 has a collapsable toolbar, this PR aim to replace our third party dependency and use the native one.
Before any merge we should also update
verification-metadata.xml
to remove the old dependency.This change is