Fix: Tab switcher UI issues #5736
Conversation
This reverts commit 25987d5.
| @@ -92,6 +92,18 @@ | |||
| app:layout_constraintStart_toStartOf="parent" | |||
| app:layout_constraintTop_toBottomOf="@id/title" /> | |||
|
|
|||
| <ImageView | |||
There was a problem hiding this comment.
❓Why do we need a new ImageView to load the dax logo into? What was stopping us from using the existing tabPreview ImageView and adjusting as needed?
There was a problem hiding this comment.
The webpage previews need to use matrix scaling, which doesn't work for the logo. I thought it'd be complicating things trying to change it on the fly when views are recycled. But after giving it another thought it's not that bad: Use a single ImageView for all tabs
| @@ -219,6 +219,7 @@ open class BrowserActivity : DuckDuckGoActivity() { | |||
| if (wasSwipingStarted) { | |||
| wasSwipingStarted = false | |||
|
|
|||
| currentTab?.onTabSwipedAway() | |||
| viewModel.onTabsSwiped() | |||
There was a problem hiding this comment.
💡Could we not make this part of onTabPageSwiped? Feels like this belongs there as well
| app:layout_constraintHorizontal_bias="0.0" | ||
| app:typography="h5" | ||
| tools:text="Slashdot" /> | ||
| tools:text="Slashdot test tes tes tes tes tes tew tes tes tes tes tes tes tes" /> | ||
|
|
There was a problem hiding this comment.
🔍Maybe we can reduce this a bit? 😅
| android:layout_marginEnd="7dp" | ||
| android:layout_marginBottom="10dp"> | ||
| android:layout_marginHorizontal="@dimen/keyline_2" | ||
| android:layout_marginVertical="6dp" |
There was a problem hiding this comment.
As discussed on MM, it's probably worth following up and finding out why our List item does not currently match the designs and if there is any good reason why 👍
| app:layout_constraintTop_toTopOf="parent" /> | ||
| android:src="@drawable/ic_dax_icon" | ||
| android:layout_marginStart="@dimen/keyline_2" | ||
| app:layout_constraintTop_toTopOf="@id/close" |
There was a problem hiding this comment.
❓Shouldn't this be keyline_3 (12dp)?
There was a problem hiding this comment.
I don't think so. It seems like the 12dp margin in the spec is used to compensate for the narrow icon used. Yes. Fixed.
Task/Issue URL: https://app.asana.com/0/1207418217763355/1209581022862153
Description
This PR fixes a couple of minor UI issues in the tab switcher:
Steps to test this PR
Tab switcher item layout
Tab switcher item layout
Tab preview image