For #21360 - Added toggle for search term tab groups #21615
Conversation
| @@ -0,0 +1,10 @@ | |||
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | |||
There was a problem hiding this comment.
Nice, thanks for adding this.
There was a problem hiding this comment.
Please also include the license
There was a problem hiding this comment.
Good catch. I don't think my original icon have it either. Since we need this change in, @MozillaNoah can you please create another PR to add license on both ic_all_tabs? Thanks
MozillaNoah
added
the
Feature:SearchTermTabs
MozillaNoah
added
the
pr:needs-landing-squashed
|
The UI test failures say: There is no video since the test ends too soon, so my suspicion is that since we've added new items to this screen, we now need to scroll down in the test. Something like this patch: diff --git a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt
index 405af4bcf..4c8ec5359 100644
--- a/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt
+++ b/app/src/androidTest/java/org/mozilla/fenix/ui/robots/SettingsSubMenuTabsRobot.kt
@@ -16,6 +16,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.platform.app.InstrumentationRegistry
import androidx.test.uiautomator.UiDevice
import org.hamcrest.CoreMatchers.allOf
+import org.mozilla.fenix.helpers.TestHelper.scrollToElementByText
import org.mozilla.fenix.helpers.click
/**
@@ -67,6 +68,8 @@ private fun assertCloseTabsOptions() {
}
private fun assertStartOnHomeOptions() {
+ // Scroll to ensure all the items are visible.
+ scrollToElementByText("Never")
startOnHomeHeading()
.check(matches(withEffectiveVisibility(ViewMatchers.Visibility.VISIBLE)))
afterFourHoursToggle()
|
|
@jonalmeida giving this a try, thank you! |
|
If you can't easily figure this test failure out you can add an |
| @@ -58,6 +59,10 @@ class TabsSettingsFragment : PreferenceFragmentCompat() { | |||
| // pref_key_tab_view_grid and look into using the native RadioGroup in the future. | |||
| listRadioButton = requirePreference(R.string.pref_key_tab_view_list_do_not_use) | |||
| gridRadioButton = requirePreference(R.string.pref_key_tab_view_grid) | |||
| searchTermTabGroups = requirePreference<SwitchPreference>(R.string.pref_key_search_term_tab_groups).also { | |||
There was a problem hiding this comment.
@MozillaNoah We have to make sure we only show this preference when the feature flag is turned on. Did you verify this? I think this needs a it.visible = FeatureFlags.tabGroupFeature?
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks for catching this. I did it for the inactive tabs toggle but not here. Will update shortly.
csadilek
removed
the
pr:needs-landing-squashed
MozillaNoah
added
the
pr:needs-landing-squashed
…ozilla-mobile#21615) * For mozilla-mobile#21360 - Added toggle for search term tab groups * For mozilla-mobile#21360 - Lint cleanup * PR: Added missing licenses and possibly fixed UI test * PR: Added a "scrollTo" to potentially fix a UI test * PR: Added potential fix for alwaysStartOnHomeTest * PR: Added temporary ignore to alwaysStartOnHomeTest * PR: added missing ignore comment * For mozilla-mobile#21360 - Added missing feature flag driven visibility logic Co-authored-by: Sebastian Kaspari <s.kaspari@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
#21360
Pull Request checklist
To download an APK when reviewing a PR: