-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Multi-selection: Add new FAB to the tab switcher #5467
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
Multi-selection: Add new FAB to the tab switcher #5467
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
0b382b2 to
8492b52
Compare
67aa25e to
c303d4b
Compare
8492b52 to
c4c3cf8
Compare
5916903 to
79bc6aa
Compare
c4c3cf8 to
c3270a9
Compare
79bc6aa to
2acd9ec
Compare
c3270a9 to
e6e2148
Compare
d85d6d1 to
6c267e1
Compare
e6e2148 to
ff15dd9
Compare
6c267e1 to
ae833dd
Compare
ae833dd to
88714c8
Compare
ff15dd9 to
02fb482
Compare
02fb482 to
b238763
Compare
3987163 to
195a3f0
Compare
eab74f4 to
3d62cf6
Compare
3d62cf6 to
01126c4
Compare
195a3f0 to
057e4d2
Compare
25ddd72 to
c39f956
Compare
3536b27 to
4b5cef8
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.
Looks good, just need confirmation on the testing step about whether the FAB should hide or not 👍
| private fun updateFabType(fabType: TabSwitcherViewModel.ViewState.FabType) { | ||
| when (fabType) { | ||
| TabSwitcherViewModel.ViewState.FabType.NEW_TAB -> { | ||
| tabsFab.icon = AppCompatResources.getDrawable(this, com.duckduckgo.mobile.android.R.drawable.ic_add_24) |
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.
🔍 Add an import for the R class
|
|
||
| <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" | ||
| xmlns:tools="http://schemas.android.com/tools" | ||
| <androidx.coordinatorlayout.widget.CoordinatorLayout xmlns:android="http://schemas.android.com/apk/res/android" |
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.
❓ If we're switching to a CoordinatorLayout, do we need the LinearLayout anymore?
I think app:layout_behavior="@string/appbar_scrolling_view_behavior" on the RecyclerView should take care of offsetting the RecyclerView from the Toolbar, from what I remember 😅
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.
TIL! 😁
| <item name="android:paddingStart">@dimen/keyline_2</item> | ||
| </style> | ||
|
|
||
| <style name="Widget.DuckDuckGo.ExtensibleFloatingActionButton" parent="Widget.MaterialComponents.ExtendedFloatingActionButton"> |
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.
❓ Are we calling it "Extensible" in our design system rather than "Extended"? 😁
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.
Looks good, just need confirmation on the testing step about whether the FAB should hide or not 👍
c39f956 to
66bb476
Compare
4b5cef8 to
32bdd14
Compare
|
Thanks for the review! 🙇♂️ |
32bdd14 to
cf6b2b2
Compare
7e86453 to
8b1c367
Compare
cf6b2b2 to
d7318f0
Compare
8b1c367 to
f237d70
Compare
096932e
into
feature/ondrej/tab-multi-selection
Task/Issue URL: https://app.asana.com/0/72649045549333/1207969573539951 ### Description This PR adds the FAB to the tab switcher. It is hidden under a feature flag, which must be enabled first. This PR demonstrates the 2 states of the FAB that will be used later in the multi-selection -- "New Tab" & "Close Tabs". Clicking the button will not actually change this in the actual implementation. ### Steps to test this PR - [x] Go to Settings -> Feature Flag Inventory - [x] Type "multi" - [x] Enable the feature flag - [x] Leave the tab switcher activity if there already - [x] Open the tab switcher activity - [x] Notice the FAB is displayed - [x] Verify the `+` icon is shown and the title is "New Tab" - [x] Scroll up & down - [x] Verify that the FAB shrink while scrolling down - [x] Tap on the FAB - [x] Verify the icon changes to `x` and title says "Close Tabs"

Task/Issue URL: https://app.asana.com/0/72649045549333/1207969573539951
Description
This PR adds the FAB to the tab switcher. It is hidden under a feature flag, which must be enabled first.
This PR demonstrates the 2 states of the FAB that will be used later in the multi-selection -- "New Tab" & "Close Tabs". Clicking the button will not actually change this in the actual implementation.
Steps to test this PR
+icon is shown and the title is "New Tab"xand title says "Close Tabs"