Skip to content

Commit d634b8a

Browse files
CrisBarreiroCrisBarreiro
committed
Merge branch 'hotfix/5.275.1'
2 parents 1def3bf + a47a19e commit d634b8a

11 files changed

Lines changed: 62 additions & 14 deletions

File tree

‎PixelDefinitions/pixels/definitions/browser_menu.json5‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,12 @@
2121
"triggers": ["other"],
2222
"suffixes": ["form_factor"],
2323
"parameters": ["appVersion", "vpnMenuItemStatus"]
24+
},
25+
"m_nav_bookmarks_menu_item_pressed": {
26+
"description": "Fires when the user taps the Bookmarks menu item in the browser menu",
27+
"owners": ["GerardPaligot"],
28+
"triggers": ["other"],
29+
"suffixes": ["form_factor"],
30+
"parameters": ["appVersion"]
2431
}
2532
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4914,6 +4914,7 @@ class BrowserTabViewModel @Inject constructor(
49144914
}
49154915

49164916
fun onBookmarksMenuItemClicked() {
4917+
pixel.fire(AppPixelName.MENU_ACTION_BOOKMARKS_PRESSED)
49174918
launchBookmarksActivity()
49184919
}
49194920

‎app/src/main/java/com/duckduckgo/app/browser/menu/BrowserMenuHighlightState.kt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import kotlinx.coroutines.flow.SharingStarted
2727
import kotlinx.coroutines.flow.StateFlow
2828
import kotlinx.coroutines.flow.combine
2929
import kotlinx.coroutines.flow.flowOn
30+
import kotlinx.coroutines.flow.map
3031
import kotlinx.coroutines.flow.stateIn
3132
import javax.inject.Inject
3233

@@ -45,13 +46,15 @@ interface BrowserMenuHighlightState {
4546
class RealBrowserMenuHighlightState @Inject constructor(
4647
additionalDefaultBrowserPrompts: AdditionalDefaultBrowserPrompts,
4748
downloadMenuStateProvider: DownloadMenuStateProvider,
49+
browserMenuDisplayRepository: BrowserMenuDisplayRepository,
4850
@AppCoroutineScope appCoroutineScope: CoroutineScope,
4951
dispatcherProvider: DispatcherProvider,
5052
) : BrowserMenuHighlightState {
5153
override val shouldHighlight: StateFlow<Boolean> = combine(
5254
additionalDefaultBrowserPrompts.highlightPopupMenu,
5355
downloadMenuStateProvider.hasNewDownloadFlow,
54-
) { defaultBrowserHighlight, hasNewDownload ->
55-
defaultBrowserHighlight || hasNewDownload
56+
browserMenuDisplayRepository.browserMenuState.map { it.isEnabled },
57+
) { defaultBrowserHighlight, hasNewDownload, isBottomSheetMenu ->
58+
defaultBrowserHighlight || (hasNewDownload && isBottomSheetMenu)
5659
}.flowOn(dispatcherProvider.io()).stateIn(appCoroutineScope, SharingStarted.Eagerly, false)
5760
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ class OmnibarLayoutViewModel @Inject constructor(
139139
shouldUpdateTabsCount = tabs.size != state.tabCount && tabs.isNotEmpty(),
140140
tabCount = tabs.size,
141141
hasUnreadTabs = tabs.firstOrNull { !it.viewed } != null,
142-
showBrowserMenuHighlight = shouldHighlightMenu,
142+
showBrowserMenuHighlight = shouldHighlightMenu && state.viewMode !is CustomTab,
143143
viewMode = getViewMode(state),
144144
isAddressBarTrackersAnimationEnabled = isAddressBarTrackersAnimationEnabled,
145145
)

‎app/src/main/java/com/duckduckgo/app/global/api/PixelParamRemovalInterceptor.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ object PixelInterceptorPixelsRequiringDataCleaning : PixelParamRemovalPlugin {
228228
AppPixelName.GET_DESKTOP_BROWSER_SHARE_DOWNLOAD_LINK_CLICK.pixelName to PixelParameter.removeAtb(),
229229
AppPixelName.GET_DESKTOP_BROWSER_LINK_CLICK.pixelName to PixelParameter.removeAtb(),
230230
AppPixelName.MENU_ACTION_VPN_PRESSED.pixelName to PixelParameter.removeAtb(),
231+
AppPixelName.MENU_ACTION_BOOKMARKS_PRESSED.pixelName to PixelParameter.removeAtb(),
231232
SqlCipherPixelName.LIBRARY_LOAD_FAILURE_SQLCIPHER.pixelName to PixelParameter.removeAtb(),
232233
SqlCipherPixelName.LIBRARY_LOAD_TIMEOUT_SQLCIPHER.pixelName to PixelParameter.removeAtb(),
233234
AppPixelName.FORGET_ALL_PRESSED_BROWSING_DAILY.pixelName to PixelParameter.removeAtb(),

‎app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ enum class AppPixelName(override val pixelName: String) : Pixel.PixelName {
269269
MENU_ACTION_NAVIGATE_BACK_PRESSED("m_nav_nb_p"),
270270
MENU_ACTION_ADD_BOOKMARK_PRESSED("m_nav_ab_p"),
271271
MENU_ACTION_EDIT_BOOKMARK_PRESSED("m_nav_eb_p"),
272+
MENU_ACTION_BOOKMARKS_PRESSED("m_nav_bookmarks_menu_item_pressed"),
272273
MENU_ACTION_ADD_FAVORITE_PRESSED("m_nav_af_p"),
273274
MENU_ACTION_REMOVE_FAVORITE_PRESSED("m_nav_rf_p"),
274275
MENU_ACTION_SHARE_PRESSED("m_nav_sh_p"),

‎app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4486,6 +4486,12 @@ class BrowserTabViewModelTest {
44864486
verify(mockPixel).fire(AppPixelName.MENU_ACTION_PRINT_PRESSED)
44874487
}
44884488

4489+
@Test
4490+
fun whenUserClicksBookmarksMenuItemThenPixelIsSent() {
4491+
testee.onBookmarksMenuItemClicked()
4492+
verify(mockPixel).fire(AppPixelName.MENU_ACTION_BOOKMARKS_PRESSED)
4493+
}
4494+
44894495
@Test
44904496
fun whenSubmittedQueryAndNavigationStateIsNullAndNeverPreviouslyLoadedSiteThenResetHistoryCommandNotSent() {
44914497
whenever(mockOmnibarConverter.convertQueryToUrl("nytimes.com", null)).thenReturn("nytimes.com")

‎app/src/test/java/com/duckduckgo/app/browser/menu/RealBrowserMenuHighlightStateTest.kt‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,26 @@ class RealBrowserMenuHighlightStateTest {
3838

3939
private val downloadMenuStateProvider: DownloadMenuStateProvider = mock()
4040

41+
private val browserMenuDisplayRepository: BrowserMenuDisplayRepository = mock()
42+
4143
private val highlightPopupMenuFlow = MutableStateFlow(false)
4244

4345
private val hasNewDownloadFlow = MutableStateFlow(false)
4446

47+
private val browserMenuStateFlow = MutableStateFlow(BrowserMenuDisplayState(hasOption = false, isEnabled = true))
48+
4549
private lateinit var testee: RealBrowserMenuHighlightState
4650

4751
@Before
4852
fun setUp() {
4953
whenever(additionalDefaultBrowserPrompts.highlightPopupMenu).thenReturn(highlightPopupMenuFlow)
5054
whenever(downloadMenuStateProvider.hasNewDownloadFlow).thenReturn(hasNewDownloadFlow)
55+
whenever(browserMenuDisplayRepository.browserMenuState).thenReturn(browserMenuStateFlow)
5156

5257
testee = RealBrowserMenuHighlightState(
5358
additionalDefaultBrowserPrompts = additionalDefaultBrowserPrompts,
5459
downloadMenuStateProvider = downloadMenuStateProvider,
60+
browserMenuDisplayRepository = browserMenuDisplayRepository,
5561
appCoroutineScope = coroutineTestRule.testScope,
5662
dispatcherProvider = coroutineTestRule.testDispatcherProvider,
5763
)
@@ -78,7 +84,7 @@ class RealBrowserMenuHighlightStateTest {
7884
}
7985

8086
@Test
81-
fun `when new download is true then shouldHighlight is true`() = runTest {
87+
fun `when new download is true and bottom sheet menu enabled then shouldHighlight is true`() = runTest {
8288
testee.shouldHighlight.test {
8389
assertFalse(awaitItem())
8490

@@ -89,6 +95,29 @@ class RealBrowserMenuHighlightStateTest {
8995
}
9096
}
9197

98+
@Test
99+
fun `when new download is true but bottom sheet menu disabled then shouldHighlight is false`() = runTest {
100+
browserMenuStateFlow.value = BrowserMenuDisplayState(hasOption = false, isEnabled = false)
101+
102+
testee = RealBrowserMenuHighlightState(
103+
additionalDefaultBrowserPrompts = additionalDefaultBrowserPrompts,
104+
downloadMenuStateProvider = downloadMenuStateProvider,
105+
browserMenuDisplayRepository = browserMenuDisplayRepository,
106+
appCoroutineScope = coroutineTestRule.testScope,
107+
dispatcherProvider = coroutineTestRule.testDispatcherProvider,
108+
)
109+
110+
testee.shouldHighlight.test {
111+
assertFalse(awaitItem())
112+
113+
hasNewDownloadFlow.value = true
114+
// still false because bottom sheet menu is not enabled
115+
expectNoEvents()
116+
117+
cancelAndIgnoreRemainingEvents()
118+
}
119+
}
120+
92121
@Test
93122
fun `when both sources are true then shouldHighlight is true`() = runTest {
94123
testee.shouldHighlight.test {

‎app/version/version.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
VERSION=5.275.0
1+
VERSION=5.275.1

‎browser/browser-ui/src/main/java/com/duckduckgo/browser/ui/browsermenu/BrowserMenuBottomSheet.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,6 @@ class BrowserMenuBottomSheet(
524524
}
525525

526526
companion object {
527-
internal const val PEEK_HEIGHT_PERCENT = 80
527+
internal const val PEEK_HEIGHT_PERCENT = 90
528528
}
529529
}

0 commit comments

Comments
 (0)