Skip to content

Commit 72c8018

Browse files
malmsteinmalmstein
authored
Visual desing: Design Review Feedback Take 1 (#6078)
Task/Issue URL: https://app.asana.com/1/137249556945/task/1210261391529122?focus=true ### Description Fixes 4 feedback items from the design review. See tasks for more detail --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1210261391529122
1 parent e2e2be3 commit 72c8018

29 files changed

+288
-107
lines changed

‎app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2664,7 +2664,6 @@ class BrowserTabFragment :
26642664
val topOfPage = scrollY == 0
26652665

26662666
omnibar.setContentCanScroll(canScrollUp, canScrollDown, topOfPage)
2667-
browserNavigationBarIntegration.setContentCanScroll(canScrollUp, canScrollDown, topOfPage)
26682667
}
26692668
}
26702669

‎app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2832,9 +2832,7 @@ class BrowserTabViewModel @Inject constructor(
28322832

28332833
private fun showOrHideKeyboard(cta: Cta?) {
28342834
// we hide the keyboard when showing a DialogCta and HomeCta type in the home screen otherwise we show it
2835-
// we also don't want to automatically show keyboard when bottom nav bar is enabled because it overlaps and hides the navigation/tabs buttons
2836-
val isBottomNavigationBar = visualDesignExperimentDataStore.isExperimentEnabled.value
2837-
val shouldHideKeyboard = cta is HomePanelCta || cta is DaxBubbleCta.DaxPrivacyProCta || isBottomNavigationBar
2835+
val shouldHideKeyboard = cta is HomePanelCta || cta is DaxBubbleCta.DaxPrivacyProCta
28382836
command.value = if (shouldHideKeyboard) HideKeyboard else ShowKeyboard
28392837
}
28402838

‎app/src/main/java/com/duckduckgo/app/browser/navigation/bar/BrowserNavigationBarViewIntegration.kt

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarView
2323
import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarView.ViewMode.Browser
2424
import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarView.ViewMode.NewTab
2525
import com.duckduckgo.app.browser.omnibar.Omnibar
26-
import com.duckduckgo.app.browser.omnibar.model.OmnibarPosition
2726
import com.duckduckgo.common.ui.experiments.visual.store.VisualDesignExperimentDataStore
2827
import com.duckduckgo.common.ui.view.gone
2928
import com.duckduckgo.common.ui.view.show
@@ -110,14 +109,4 @@ class BrowserNavigationBarViewIntegration(
110109
private fun onDisabled() {
111110
keyboardVisibilityJob?.cancel()
112111
}
113-
114-
fun setContentCanScroll(
115-
canScrollUp: Boolean,
116-
canScrollDown: Boolean,
117-
topOfPage: Boolean,
118-
) {
119-
if (omnibar.omnibarPosition == OmnibarPosition.TOP) {
120-
navigationBarView.showShadow(canScrollDown)
121-
}
122-
}
123112
}

‎app/src/main/java/com/duckduckgo/app/browser/navigation/bar/view/BrowserNavigationBarView.kt

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import androidx.lifecycle.findViewTreeViewModelStoreOwner
3333
import androidx.lifecycle.lifecycleScope
3434
import com.duckduckgo.anvil.annotations.InjectWith
3535
import com.duckduckgo.app.browser.PulseAnimation
36+
import com.duckduckgo.app.browser.R
3637
import com.duckduckgo.app.browser.databinding.ViewBrowserNavigationBarBinding
3738
import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarViewModel.Command
3839
import com.duckduckgo.app.browser.navigation.bar.view.BrowserNavigationBarViewModel.Command.NotifyAutofillButtonClicked
@@ -66,6 +67,16 @@ class BrowserNavigationBarView @JvmOverloads constructor(
6667
defStyle: Int = 0,
6768
) : FrameLayout(context, attrs, defStyle), AttachedBehavior {
6869

70+
private var showShadows: Boolean = false
71+
72+
init {
73+
context.theme.obtainStyledAttributes(attrs, R.styleable.BrowserNavigationBarView, defStyle, 0)
74+
.apply {
75+
showShadows = getBoolean(R.styleable.BrowserNavigationBarView_showShadows, true)
76+
recycle()
77+
}
78+
}
79+
6980
override fun setVisibility(visibility: Int) {
7081
val isVisibilityUpdated = this.visibility != visibility
7182

@@ -127,10 +138,6 @@ class BrowserNavigationBarView @JvmOverloads constructor(
127138
}
128139
}
129140

130-
fun showShadow(show: Boolean) {
131-
binding.shadowView.isVisible = show
132-
}
133-
134141
override fun onAttachedToWindow() {
135142
AndroidSupportInjection.inject(this)
136143
super.onAttachedToWindow()
@@ -189,6 +196,7 @@ class BrowserNavigationBarView @JvmOverloads constructor(
189196
}
190197

191198
private fun renderView(viewState: ViewState) {
199+
binding.shadowView.isVisible = showShadows
192200
binding.root.isVisible = viewState.isVisible
193201

194202
binding.newTabButton.isVisible = viewState.newTabButtonVisible

‎app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModel.kt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -725,12 +725,12 @@ class OmnibarLayoutViewModel @Inject constructor(
725725

726726
fun onNewTabScrollingStateChanged(scrollingState: Decoration.NewTabScrollingState) {
727727
val viewMode = viewState.value.viewMode
728-
if (viewMode is NewTab) {
729-
_viewState.update {
730-
it.copy(
731-
showShadows = (scrollingState.canScrollUp || scrollingState.canScrollDown) && !scrollingState.topOfPage,
732-
)
733-
}
734-
}
728+
// if (viewMode is NewTab) {
729+
// _viewState.update {
730+
// it.copy(
731+
// showShadows = (scrollingState.canScrollUp || scrollingState.canScrollDown) && !scrollingState.topOfPage,
732+
// )
733+
// }
734+
// }
735735
}
736736
}

‎app/src/main/java/com/duckduckgo/app/browser/omnibar/experiments/FadeOmnibarLayout.kt

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import android.content.Context
2121
import android.util.AttributeSet
2222
import android.view.View
2323
import android.view.ViewGroup
24-
import android.view.ViewOutlineProvider
2524
import android.view.ViewTreeObserver.OnGlobalLayoutListener
2625
import android.view.animation.DecelerateInterpolator
2726
import android.widget.ImageView
@@ -91,6 +90,9 @@ class FadeOmnibarLayout @JvmOverloads constructor(
9190
private val toolbarContainerPaddingTopWhenAtBottom by lazy {
9291
resources.getDimensionPixelSize(CommonR.dimen.experimentalToolbarContainerPaddingTopWhenAtBottom)
9392
}
93+
private val toolbarContainerPaddingHorizontalWhenAtBottom by lazy {
94+
resources.getDimensionPixelSize(CommonR.dimen.experimentalToolbarContainerPaddingHorizontalWhenAtBottom)
95+
}
9496
private val omnibarCardMarginHorizontal by lazy { resources.getDimensionPixelSize(CommonR.dimen.experimentalOmnibarCardMarginHorizontal) }
9597
private val omnibarCardMarginTop by lazy { resources.getDimensionPixelSize(CommonR.dimen.experimentalOmnibarCardMarginTop) }
9698
private val omnibarCardMarginBottom by lazy { resources.getDimensionPixelSize(CommonR.dimen.experimentalOmnibarCardMarginBottom) }
@@ -127,9 +129,11 @@ class FadeOmnibarLayout @JvmOverloads constructor(
127129
// When omnibar is at the bottom, we're adding an additional space at the top
128130
toolbarContainer.updatePadding(
129131
top = toolbarContainerPaddingTopWhenAtBottom,
132+
right = toolbarContainerPaddingHorizontalWhenAtBottom,
133+
left = toolbarContainerPaddingHorizontalWhenAtBottom,
130134
)
131135
// at the same time, we remove that space from the navigation bar which now sits below the omnibar
132-
navBar.findViewById<LinearLayout>(R.id.rootView).updatePadding(
136+
navBar.findViewById<LinearLayout>(R.id.barView).updatePadding(
133137
top = 0,
134138
)
135139

@@ -334,11 +338,11 @@ class FadeOmnibarLayout @JvmOverloads constructor(
334338
}
335339

336340
private fun renderShadows(showShadows: Boolean) {
337-
outlineProvider = if (showShadows) {
338-
ViewOutlineProvider.BACKGROUND
339-
} else {
340-
null
341-
}
341+
// outlineProvider = if (showShadows) {
342+
// ViewOutlineProvider.BACKGROUND
343+
// } else {
344+
// null
345+
// }
342346
}
343347

344348
companion object {

‎app/src/main/res/drawable/background_navigation_bar_shadow.xml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
android:shape="rectangle">
1818
<gradient
1919
android:angle="270"
20-
android:startColor="#00000000"
21-
android:endColor="#20000000"
20+
android:endColor="#00000000"
21+
android:startColor="#1A000000"
2222
android:type="linear" />
2323
</shape>

‎app/src/main/res/layout-w600dp/view_browser_navigation_bar.xml

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
<?xml version="1.0" encoding="utf-8"?><!--
2+
~ Copyright (c) 2025 DuckDuckGo
3+
~
4+
~ Licensed under the Apache License, Version 2.0 (the "License");
5+
~ you may not use this file except in compliance with the License.
6+
~ You may obtain a copy of the License at
7+
~
8+
~ http://www.apache.org/licenses/LICENSE-2.0
9+
~
10+
~ Unless required by applicable law or agreed to in writing, software
11+
~ distributed under the License is distributed on an "AS IS" BASIS,
12+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
~ See the License for the specific language governing permissions and
14+
~ limitations under the License.
15+
-->
16+
17+
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
18+
xmlns:app="http://schemas.android.com/apk/res-auto"
19+
xmlns:tools="http://schemas.android.com/tools"
20+
android:id="@+id/rootView"
21+
android:layout_width="match_parent"
22+
android:layout_height="wrap_content">
23+
24+
<LinearLayout
25+
android:id="@+id/barView"
26+
android:layout_width="match_parent"
27+
android:layout_height="wrap_content"
28+
android:background="?attr/preferredNavigationBarColor"
29+
android:gravity="center"
30+
android:orientation="horizontal"
31+
android:paddingTop="@dimen/experimentalNavigationBarPaddingTopWhenStandalone"
32+
android:paddingBottom="@dimen/experimentalNavigationBarPaddingBottom"
33+
app:layout_constraintTop_toTopOf="parent"
34+
tools:parentTag="androidx.coordinatorlayout.widget.CoordinatorLayout">
35+
36+
<Space
37+
android:layout_width="0dp"
38+
android:layout_height="match_parent"
39+
android:layout_weight="1" />
40+
41+
42+
<Space
43+
android:layout_width="0dp"
44+
android:layout_height="match_parent"
45+
android:layout_weight="1" />
46+
47+
48+
<FrameLayout
49+
android:layout_width="wrap_content"
50+
android:layout_height="wrap_content">
51+
52+
<FrameLayout
53+
android:id="@+id/newTabButton"
54+
android:layout_width="@dimen/bottomNavIconContainer"
55+
android:layout_height="@dimen/bottomNavIconContainer"
56+
android:background="@drawable/selectable_item_experimental_background"
57+
android:visibility="gone"
58+
tools:visibility="visible">
59+
60+
<ImageView
61+
android:id="@+id/newTabButtonImageView"
62+
android:layout_width="@dimen/bottomNavIcon"
63+
android:layout_height="@dimen/bottomNavIcon"
64+
android:layout_gravity="center"
65+
android:contentDescription="@string/newTabMenuItem"
66+
android:scaleType="center"
67+
android:src="@drawable/ic_add_24" />
68+
</FrameLayout>
69+
70+
<FrameLayout
71+
android:id="@+id/autofillButton"
72+
android:layout_width="@dimen/bottomNavIconContainer"
73+
android:layout_height="@dimen/bottomNavIconContainer"
74+
android:background="@drawable/selectable_item_experimental_background"
75+
android:visibility="gone"
76+
tools:visibility="visible">
77+
78+
<ImageView
79+
android:id="@+id/autofillButtonImageView"
80+
android:layout_width="@dimen/bottomNavIcon"
81+
android:layout_height="@dimen/bottomNavIcon"
82+
android:layout_gravity="center"
83+
android:contentDescription="@string/autofillManagementScreenTitle"
84+
android:scaleType="center"
85+
android:src="@drawable/ic_key_24" />
86+
</FrameLayout>
87+
88+
</FrameLayout>
89+
90+
<Space
91+
android:layout_width="0dp"
92+
android:layout_height="match_parent"
93+
android:layout_weight="1" />
94+
95+
<FrameLayout
96+
android:id="@+id/bookmarksButton"
97+
android:layout_width="@dimen/bottomNavIconContainer"
98+
android:layout_height="@dimen/bottomNavIconContainer"
99+
android:background="@drawable/selectable_item_experimental_background">
100+
101+
<ImageView
102+
android:id="@+id/bookmarksImageView"
103+
android:layout_width="@dimen/bottomNavIcon"
104+
android:layout_height="@dimen/bottomNavIcon"
105+
android:layout_gravity="center"
106+
android:contentDescription="@string/bookmarksMenuTitle"
107+
android:scaleType="center"
108+
android:src="@drawable/ic_bookmarks_24" />
109+
</FrameLayout>
110+
111+
<Space
112+
android:layout_width="0dp"
113+
android:layout_height="match_parent"
114+
android:layout_weight="1" />
115+
116+
<FrameLayout
117+
android:id="@+id/fireButton"
118+
android:layout_width="@dimen/bottomNavIconContainer"
119+
android:layout_height="@dimen/bottomNavIconContainer"
120+
android:background="@drawable/selectable_item_experimental_background">
121+
122+
<ImageView
123+
android:id="@+id/fireIconImageView"
124+
android:layout_width="@dimen/bottomNavIcon"
125+
android:layout_height="@dimen/bottomNavIcon"
126+
android:layout_gravity="center"
127+
android:contentDescription="@string/fireMenu"
128+
android:scaleType="center"
129+
android:src="@drawable/ic_fire_24" />
130+
</FrameLayout>
131+
132+
<Space
133+
android:layout_width="0dp"
134+
android:layout_height="match_parent"
135+
android:layout_weight="1" />
136+
137+
<com.duckduckgo.app.browser.tabswitcher.ExperimentalTabSwitcherButton
138+
android:id="@+id/tabsButton"
139+
android:layout_width="@dimen/bottomNavIconContainer"
140+
android:layout_height="@dimen/bottomNavIconContainer" />
141+
142+
<Space
143+
android:layout_width="0dp"
144+
android:layout_height="match_parent"
145+
android:layout_weight="1" />
146+
147+
<FrameLayout
148+
android:id="@+id/menuButton"
149+
android:layout_width="@dimen/bottomNavIconContainer"
150+
android:layout_height="@dimen/bottomNavIconContainer"
151+
android:background="@drawable/selectable_item_experimental_background">
152+
153+
<ImageView
154+
android:id="@+id/menuIconImageView"
155+
android:layout_width="@dimen/bottomNavIcon"
156+
android:layout_height="@dimen/bottomNavIcon"
157+
android:layout_gravity="center"
158+
android:contentDescription="@string/browserPopupMenu"
159+
android:scaleType="center"
160+
android:src="@drawable/ic_menu_vertical_24" />
161+
</FrameLayout>
162+
163+
<Space
164+
android:layout_width="0dp"
165+
android:layout_height="match_parent"
166+
android:layout_weight="1" />
167+
168+
<Space
169+
android:layout_width="0dp"
170+
android:layout_height="match_parent"
171+
android:layout_weight="1" />
172+
173+
174+
</LinearLayout>
175+
176+
<View
177+
android:id="@+id/shadowView"
178+
android:layout_width="match_parent"
179+
android:layout_height="2dp"
180+
android:background="@drawable/background_navigation_bar_shadow"
181+
app:layout_constraintEnd_toEndOf="parent"
182+
app:layout_constraintStart_toStartOf="parent"
183+
app:layout_constraintTop_toTopOf="parent" />
184+
185+
186+
</androidx.constraintlayout.widget.ConstraintLayout>

‎app/src/main/res/layout/content_settings_protections.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
android:layout_width="match_parent"
4848
android:layout_height="wrap_content"
4949
app:indicatorStatus="on"
50-
app:leadingIcon="@drawable/ic_shield_check_color_24"
50+
app:leadingIcon="@drawable/ic_shield_color_24"
5151
app:primaryText="@string/settingsWebTrackingProtectionTitle" />
5252

5353
<com.duckduckgo.common.ui.view.listitem.SettingsListItem

‎app/src/main/res/layout/cookie_scene_1.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,15 @@
1616

1717
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
1818
android:layout_width="wrap_content"
19-
android:layout_height="@dimen/omnibarCookieAnimationBannerHeight"
19+
android:layout_height="?attr/cookiesAnimationHeight"
2020
xmlns:tools="http://schemas.android.com/tools" xmlns:app="http://schemas.android.com/apk/res-auto">
2121

2222
<com.duckduckgo.common.ui.view.text.DaxTextView
2323
android:id="@+id/cookiesManagedText"
2424
app:typography="body1"
2525
android:layout_width="wrap_content"
2626
android:layout_height="match_parent"
27+
android:layout_marginStart="8dp"
2728
android:paddingStart="16dp"
2829
android:paddingEnd="16dp"
2930
android:maxLines="1"

0 commit comments

Comments
 (0)