Skip to content
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

New Layout - Labs Flag (to replace feature flag) #7038

Merged
merged 16 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7038.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Adds New App Layout into Labs
3 changes: 3 additions & 0 deletions library/ui-strings/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,9 @@
<string name="home_layout_preferences_sort_activity">Activity</string>
<string name="home_layout_preferences_sort_name">A - Z</string>

<string name="labs_enable_new_app_layout_title">Enable new layout</string>
<string name="labs_enable_new_app_layout_summary">A simplified Element with optional tabs</string>

<!-- Home fragment -->
<string name="invitations_header">Invites</string>
<string name="low_priority_header">Low priority</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,21 @@ import im.vector.app.espresso.tools.clickOnPreference
import im.vector.app.espresso.tools.waitUntilActivityVisible
import im.vector.app.espresso.tools.waitUntilDialogVisible
import im.vector.app.espresso.tools.waitUntilViewVisible
import im.vector.app.features.DefaultVectorFeatures
import im.vector.app.features.VectorFeatures
import im.vector.app.features.createdirect.CreateDirectRoomActivity
import im.vector.app.features.home.HomeActivity
import im.vector.app.features.onboarding.OnboardingActivity
import im.vector.app.features.settings.VectorSettingsActivity
import im.vector.app.initialSyncIdlingResource
import im.vector.app.ui.robot.settings.SettingsRobot
import im.vector.app.ui.robot.settings.labs.LabFeature
import im.vector.app.ui.robot.settings.labs.LabFeaturesPreferences
import im.vector.app.ui.robot.space.SpaceRobot
import im.vector.app.withIdlingResource
import timber.log.Timber

class ElementRobot {
private val features: VectorFeatures = DefaultVectorFeatures()

class ElementRobot(
private val labsPreferences: LabFeaturesPreferences = LabFeaturesPreferences(false)
) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can invoke the constructor in UiAllScreensSanityTest this way:

    private val elementRobot = ElementRobot(
            LabFeaturesPreferences(
                    InstrumentationRegistry.getInstrumentation()
                            .targetContext
                            .resources
                            .getBoolean(R.bool.settings_labs_new_app_layout_default)
            )
    )

fun onboarding(block: OnboardingRobot.() -> Unit) {
block(OnboardingRobot())
}
Expand Down Expand Up @@ -83,7 +82,7 @@ class ElementRobot {
}

fun settings(shouldGoBack: Boolean = true, block: SettingsRobot.() -> Unit) {
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
onView(withId((R.id.avatar))).perform(click())
} else {
openDrawer()
Expand All @@ -96,7 +95,7 @@ class ElementRobot {
}

fun newDirectMessage(block: NewDirectMessageRobot.() -> Unit) {
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
clickOn(R.id.newLayoutCreateChatButton)
waitUntilDialogVisible(withId(R.id.start_chat))
clickOn(R.id.start_chat)
Expand All @@ -111,29 +110,29 @@ class ElementRobot {
closeSoftKeyboard()
block(NewDirectMessageRobot())
pressBack()
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
pressBack() // close create dialog
}
waitUntilViewVisible(withId(R.id.roomListContainer))
}

fun newRoom(block: NewRoomRobot.() -> Unit) {
if (!features.isNewAppLayoutEnabled()) {
if (!labsPreferences.isNewAppLayoutEnabled) {
clickOn(R.id.bottom_action_rooms)
}
RoomListRobot().newRoom { block() }
if (features.isNewAppLayoutEnabled()) {
RoomListRobot(labsPreferences).newRoom { block() }
if (labsPreferences.isNewAppLayoutEnabled) {
pressBack() // close create dialog
}
waitUntilViewVisible(withId(R.id.roomListContainer))
}

fun roomList(block: RoomListRobot.() -> Unit) {
if (!features.isNewAppLayoutEnabled()) {
if (!labsPreferences.isNewAppLayoutEnabled) {
clickOn(R.id.bottom_action_rooms)
}

block(RoomListRobot())
block(RoomListRobot(labsPreferences))
waitUntilViewVisible(withId(R.id.roomListContainer))
}

Expand Down Expand Up @@ -174,7 +173,7 @@ class ElementRobot {
}

fun signout(expectSignOutWarning: Boolean) {
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
onView(withId((R.id.avatar)))
.perform(click())
waitUntilActivityVisible<VectorSettingsActivity> {
Expand Down Expand Up @@ -224,7 +223,7 @@ class ElementRobot {
}

fun space(block: SpaceRobot.() -> Unit) {
block(SpaceRobot())
block(SpaceRobot(labsPreferences))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ import im.vector.app.R
import im.vector.app.espresso.tools.waitUntilViewVisible
import im.vector.app.features.DefaultVectorFeatures
import im.vector.app.features.VectorFeatures
import im.vector.app.ui.robot.settings.labs.LabFeaturesPreferences

class NewRoomRobot(
var createdRoom: Boolean = false
var createdRoom: Boolean = false,
private val labsPreferences: LabFeaturesPreferences
) {
private val features: VectorFeatures = DefaultVectorFeatures()

fun createNewRoom(block: CreateNewRoomRobot.() -> Unit) {
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
clickOn(R.string.create_new_room)
}
waitUntilViewVisible(withId(R.id.createRoomForm))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,10 @@ import com.adevinta.android.barista.interaction.BaristaClickInteractions.clickOn
import im.vector.app.R
import im.vector.app.espresso.tools.waitUntilActivityVisible
import im.vector.app.espresso.tools.waitUntilDialogVisible
import im.vector.app.features.DefaultVectorFeatures
import im.vector.app.features.VectorFeatures
import im.vector.app.features.roomdirectory.RoomDirectoryActivity
import im.vector.app.ui.robot.settings.labs.LabFeaturesPreferences

class RoomListRobot {
private val features: VectorFeatures = DefaultVectorFeatures()
class RoomListRobot(private val labsPreferences: LabFeaturesPreferences) {

fun openRoom(roomName: String, block: RoomDetailRobot.() -> Unit) {
clickOn(roomName)
Expand All @@ -53,7 +51,7 @@ class RoomListRobot {
}

fun newRoom(block: NewRoomRobot.() -> Unit) {
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
clickOn(R.id.newLayoutCreateChatButton)
waitUntilDialogVisible(ViewMatchers.withId(R.id.create_room))
clickOn(R.id.create_room)
Expand All @@ -63,7 +61,7 @@ class RoomListRobot {
BaristaVisibilityAssertions.assertDisplayed(R.id.publicRoomsList)
}
}
val newRoomRobot = NewRoomRobot()
val newRoomRobot = NewRoomRobot(false, labsPreferences)
block(newRoomRobot)
if (!newRoomRobot.createdRoom) {
pressBack()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright (c) 2022 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package im.vector.app.ui.robot.settings.labs

data class LabFeaturesPreferences(val isNewAppLayoutEnabled: Boolean)
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ import im.vector.app.espresso.tools.waitUntilDialogVisible
import im.vector.app.espresso.tools.waitUntilViewVisible
import im.vector.app.features.DefaultVectorFeatures
import im.vector.app.features.VectorFeatures
import im.vector.app.ui.robot.settings.labs.LabFeaturesPreferences
import org.hamcrest.Matchers

class SpaceRobot {
class SpaceRobot(private val labsPreferences: LabFeaturesPreferences) {
private val features: VectorFeatures = DefaultVectorFeatures()

fun createSpace(isFirstSpace: Boolean, block: SpaceCreateRobot.() -> Unit) {
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
clickOn(R.id.newLayoutOpenSpacesButton)
if (isFirstSpace) {
waitUntilDialogVisible(ViewMatchers.withId(R.id.spaces_empty_group))
Expand All @@ -59,7 +60,7 @@ class SpaceRobot {
}

fun spaceMenu(spaceName: String, block: SpaceMenuRobot.() -> Unit) {
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
clickOn(R.id.newLayoutOpenSpacesButton)
waitUntilDialogVisible(ViewMatchers.withId(R.id.groupListView))
} else {
Expand All @@ -73,7 +74,7 @@ class SpaceRobot {

fun openMenu(spaceName: String) {
waitUntilViewVisible(ViewMatchers.withId(R.id.groupListView))
if (features.isNewAppLayoutEnabled()) {
if (labsPreferences.isNewAppLayoutEnabled) {
Espresso.onView(ViewMatchers.withId(R.id.groupListView))
.perform(
RecyclerViewActions.actionOnItem<RecyclerView.ViewHolder>(
Expand All @@ -95,7 +96,7 @@ class SpaceRobot {
}

fun selectSpace(spaceName: String) {
if (!features.isNewAppLayoutEnabled()) {
if (!labsPreferences.isNewAppLayoutEnabled) {
openDrawer()
waitUntilViewVisible(ViewMatchers.withId(R.id.groupListView))
}
Expand Down
1 change: 1 addition & 0 deletions vector-config/src/main/res/values/config-settings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

<!-- Level 1: Labs -->
<bool name="settings_labs_thread_messages_default">false</bool>
<bool name="settings_labs_new_app_layout_default">false</bool>
<bool name="settings_timeline_show_live_sender_info_visible">true</bool>
<bool name="settings_timeline_show_live_sender_info_default">false</bool>
<!-- Level 1: Advanced settings -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class DebugFeaturesStateFactory @Inject constructor(
createBooleanFeature(
label = "Enable New App Layout",
key = DebugFeatureKeys.newAppLayoutEnabled,
factory = VectorFeatures::isNewAppLayoutEnabled
factory = VectorFeatures::isNewAppLayoutFeatureEnabled
),
createBooleanFeature(
label = "Enable New Device Management",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ class DebugVectorFeatures(
override fun shouldStartDmOnFirstMessage(): Boolean = read(DebugFeatureKeys.startDmOnFirstMsg)
?: vectorFeatures.shouldStartDmOnFirstMessage()

override fun isNewAppLayoutEnabled(): Boolean = read(DebugFeatureKeys.newAppLayoutEnabled)
?: vectorFeatures.isNewAppLayoutEnabled()
override fun isNewAppLayoutFeatureEnabled(): Boolean = read(DebugFeatureKeys.newAppLayoutEnabled)
?: vectorFeatures.isNewAppLayoutFeatureEnabled()

override fun isNewDeviceManagementEnabled(): Boolean = read(DebugFeatureKeys.newDeviceManagementEnabled)
?: vectorFeatures.isNewDeviceManagementEnabled()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ abstract class VectorBaseActivity<VB : ViewBinding> : AppCompatActivity(), Maver

initUiAndData()

if (vectorFeatures.isNewAppLayoutEnabled()) {
if (vectorPreferences.isNewAppLayoutEnabled()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since both vectorFeatures.isNewAppLayoutEnabled() and vectorPreferences.isNewAppLayoutEnabled() exist now, there is a risk of using the former instead of the latter, especially now that we have many open PR on the subject. Maybe rename the later to something else to force a compilation error on existing PR.

Something like vectorFeatures.isNewAppLayoutFeatureEnabled(). Also maybe document it to tell the developer that they may want to use vectorPreferences.isNewAppLayoutEnabled() instead, to take care of the labs settings.

I admit this is quite ugly...

tryOrNull { // Add to XML theme when feature flag is removed
val toolbarBackground = MaterialColors.getColor(views.root, R.attr.vctr_toolbar_background)
window.statusBarColor = toolbarBackground
Expand Down
11 changes: 9 additions & 2 deletions vector/src/main/java/im/vector/app/features/VectorFeatures.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package im.vector.app.features

import im.vector.app.config.Config
import im.vector.app.config.OnboardingVariant
import im.vector.app.features.settings.VectorPreferences

interface VectorFeatures {

Expand All @@ -33,7 +34,13 @@ interface VectorFeatures {
fun isLocationSharingEnabled(): Boolean
fun forceUsageOfOpusEncoder(): Boolean
fun shouldStartDmOnFirstMessage(): Boolean
fun isNewAppLayoutEnabled(): Boolean

/**
* This is only to enable if the labs flag should be visible and effective.
* If on the client-side you want functionality that should be enabled with the new layout,
* use [VectorPreferences.isNewAppLayoutEnabled] instead.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

fun isNewAppLayoutFeatureEnabled(): Boolean
fun isNewDeviceManagementEnabled(): Boolean
}

Expand All @@ -50,6 +57,6 @@ class DefaultVectorFeatures : VectorFeatures {
override fun isLocationSharingEnabled() = Config.ENABLE_LOCATION_SHARING
override fun forceUsageOfOpusEncoder(): Boolean = false
override fun shouldStartDmOnFirstMessage(): Boolean = false
override fun isNewAppLayoutEnabled(): Boolean = true
override fun isNewAppLayoutFeatureEnabled(): Boolean = true
override fun isNewDeviceManagementEnabled(): Boolean = false
}
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ class HomeActivity :

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
isNewAppLayoutEnabled = vectorFeatures.isNewAppLayoutEnabled()
isNewAppLayoutEnabled = vectorPreferences.isNewAppLayoutEnabled()
analyticsScreenName = MobileScreen.ScreenName.Home
supportFragmentManager.registerFragmentLifecycleCallbacks(fragmentLifecycleCallbacks, false)
unifiedPushHelper.register(this) {
Expand All @@ -217,12 +217,13 @@ class HomeActivity :
roomListSharedActionViewModel = viewModelProvider[RoomListSharedActionViewModel::class.java]
views.drawerLayout.addDrawerListener(drawerListener)
if (isFirstCreation()) {
if (vectorFeatures.isNewAppLayoutEnabled()) {
if (vectorPreferences.isNewAppLayoutEnabled()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to unlock the drawer in the else block, since this is now a runtime setting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

views.drawerLayout.setDrawerLockMode(DrawerLayout.LOCK_MODE_LOCKED_CLOSED)
replaceFragment(views.homeDetailFragmentContainer, NewHomeDetailFragment::class.java)
} else {
replaceFragment(views.homeDetailFragmentContainer, HomeDetailFragment::class.java)
replaceFragment(views.homeDrawerFragmentContainer, HomeDrawerFragment::class.java)
views.drawerLayout.setDrawerLockMode(DrawerLayout.LOCK_MODE_UNLOCKED)
}
}

Expand Down Expand Up @@ -581,12 +582,12 @@ class HomeActivity :
}

private fun checkNewAppLayoutFlagChange() {
if (buildMeta.isDebug && vectorFeatures.isNewAppLayoutEnabled() != isNewAppLayoutEnabled) {
if (buildMeta.isDebug && vectorPreferences.isNewAppLayoutEnabled() != isNewAppLayoutEnabled) {
restart()
}
}

override fun getMenuRes() = if (vectorFeatures.isNewAppLayoutEnabled()) R.menu.menu_new_home else R.menu.menu_home
override fun getMenuRes() = if (vectorPreferences.isNewAppLayoutEnabled()) R.menu.menu_new_home else R.menu.menu_home

override fun handlePrepareMenu(menu: Menu) {
menu.findItem(R.id.menu_home_init_sync_legacy).isVisible = vectorPreferences.developerMode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class HomeActivityViewModel @AssistedInject constructor(

private fun observeReleaseNotes() = withState { state ->
// we don't want to show release notes for new users or after relogin
if (state.authenticationDescription == null && vectorFeatures.isNewAppLayoutEnabled()) {
if (state.authenticationDescription == null && vectorPreferences.isNewAppLayoutEnabled()) {
releaseNotesPreferencesStore.appLayoutOnboardingShown.onEach { isAppLayoutOnboardingShown ->
if (!isAppLayoutOnboardingShown) {
_viewEvents.post(HomeActivityViewEvents.ShowReleaseNotes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import im.vector.app.R
import im.vector.app.core.di.DefaultSharedPreferences
import im.vector.app.core.resources.BuildMeta
import im.vector.app.core.time.Clock
import im.vector.app.features.VectorFeatures
import im.vector.app.features.disclaimer.SHARED_PREF_KEY
import im.vector.app.features.home.ShortcutsHandler
import im.vector.app.features.homeserver.ServerUrlsRepository
Expand All @@ -39,6 +40,7 @@ class VectorPreferences @Inject constructor(
private val context: Context,
private val clock: Clock,
private val buildMeta: BuildMeta,
private val vectorFeatures: VectorFeatures,
) {

companion object {
Expand All @@ -63,6 +65,7 @@ class VectorPreferences @Inject constructor(
const val SETTINGS_BACKGROUND_SYNC_PREFERENCE_KEY = "SETTINGS_BACKGROUND_SYNC_PREFERENCE_KEY"
const val SETTINGS_BACKGROUND_SYNC_DIVIDER_PREFERENCE_KEY = "SETTINGS_BACKGROUND_SYNC_DIVIDER_PREFERENCE_KEY"
const val SETTINGS_LABS_PREFERENCE_KEY = "SETTINGS_LABS_PREFERENCE_KEY"
const val SETTINGS_LABS_NEW_APP_LAYOUT_KEY = "SETTINGS_LABS_NEW_APP_LAYOUT_KEY"
const val SETTINGS_CRYPTOGRAPHY_PREFERENCE_KEY = "SETTINGS_CRYPTOGRAPHY_PREFERENCE_KEY"
const val SETTINGS_CRYPTOGRAPHY_DIVIDER_PREFERENCE_KEY = "SETTINGS_CRYPTOGRAPHY_DIVIDER_PREFERENCE_KEY"
const val SETTINGS_CRYPTOGRAPHY_MANAGE_PREFERENCE_KEY = "SETTINGS_CRYPTOGRAPHY_MANAGE_PREFERENCE_KEY"
Expand Down Expand Up @@ -1151,6 +1154,14 @@ class VectorPreferences @Inject constructor(
return spaceIdsJoined?.takeIf { it.isNotEmpty() }?.split(",").orEmpty()
}

/**
* Indicates whether or not new app layout is enabled.
*/
fun isNewAppLayoutEnabled(): Boolean {
return vectorFeatures.isNewAppLayoutFeatureEnabled() &&
defaultPrefs.getBoolean(SETTINGS_LABS_NEW_APP_LAYOUT_KEY, getDefault(R.bool.settings_labs_new_app_layout_default))
}

fun showLiveSenderInfo(): Boolean {
return defaultPrefs.getBoolean(SETTINGS_TIMELINE_SHOW_LIVE_SENDER_INFO, getDefault(R.bool.settings_timeline_show_live_sender_info_default))
}
Expand Down
Loading