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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2450954
Replaces feature flag with labs flag
ericdecanini Sep 6, 2022
fbe5918
Adds changelog file
ericdecanini Sep 6, 2022
a851b0a
Fixes lint error
ericdecanini Sep 6, 2022
5e0d84b
Uses referenced default value for new layout labs flag
ericdecanini Sep 6, 2022
91b4918
Merge remote-tracking branch 'origin/feature/nfe/app_layout_hide_filt…
ericdecanini Sep 6, 2022
468c7b6
Re-adds feature flag
ericdecanini Sep 6, 2022
31a3552
Merge remote-tracking branch 'origin/develop' into feature/eric/new-l…
ericdecanini Sep 6, 2022
b7efd63
Changes vector feature preference key and function name for clarity
ericdecanini Sep 6, 2022
d77ce27
Fixes wrong use of feature flag instead of labs flag
ericdecanini Sep 6, 2022
9564c8f
Changes some preferences to be enabled or disabled by labs flag
ericdecanini Sep 6, 2022
6c23634
Unlocks the drawer layout when not new layout
ericdecanini Sep 6, 2022
afbb76f
Fixes lint error
ericdecanini Sep 7, 2022
64c8789
Merge remote-tracking branch 'origin/develop' into feature/eric/new-l…
ericdecanini Sep 7, 2022
37b2163
Merge remote-tracking branch 'origin/develop' into feature/eric/new-l…
ericdecanini Sep 7, 2022
6badbe7
robots are not checking feature flag for newAppLayout
Sep 8, 2022
2ab0343
sanity test now reads properties value to determine app layout featur…
Sep 8, 2022
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
2 changes: 1 addition & 1 deletion vector-app/src/androidTest/java/im/vector/app/ui/robot/ElementRobot.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class ElementRobot {
}

fun signout(expectSignOutWarning: Boolean) {
if (features.isNewAppLayoutEnabled()) {
if (features.isNewAppLayoutFeatureEnabled()) {
Copy link
Contributor Author

@ericdecanini ericdecanini Sep 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Now that we have it as a labs flag, we can take an approach of making the checks on one layout, enabling the labs flag with the robot and making the checks on the new version. I'll do this in #7020 as to not clutter this pr

Copy link
Member

Choose a reason for hiding this comment

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

OK, good idea! Thanks

onView(withId((R.id.avatar)))
.perform(click())
waitUntilActivityVisible<VectorSettingsActivity> {
Expand Down
2 changes: 1 addition & 1 deletion vector/src/debug/java/im/vector/app/features/debug/features/DebugFeaturesStateFactory.kt
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions vector/src/debug/java/im/vector/app/features/debug/features/DebugVectorFeatures.kt
View file
Open in desktop
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
11 changes: 9 additions & 2 deletions vector/src/main/java/im/vector/app/features/VectorFeatures.kt
View file
Open in desktop
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
}
2 changes: 1 addition & 1 deletion vector/src/main/java/im/vector/app/features/home/HomeActivityViewModel.kt
View file
Open in desktop
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 && vectorFeatures.isNewAppLayoutFeatureEnabled()) {
Copy link
Member

Choose a reason for hiding this comment

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

so we want to check the labs flag here, no?

releaseNotesPreferencesStore.appLayoutOnboardingShown.onEach { isAppLayoutOnboardingShown ->
if (!isAppLayoutOnboardingShown) {
releaseNotesPreferencesStore.setAppLayoutOnboardingShown(true)
Expand Down
4 changes: 2 additions & 2 deletions vector/src/main/java/im/vector/app/features/settings/VectorPreferences.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +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_ENABLE_NEW_LAYOUT"
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 @@ -1158,7 +1158,7 @@ class VectorPreferences @Inject constructor(
* Indicates whether or not new app layout is enabled.
*/
fun isNewAppLayoutEnabled(): Boolean {
return vectorFeatures.isNewAppLayoutEnabled() &&
return vectorFeatures.isNewAppLayoutFeatureEnabled() &&
defaultPrefs.getBoolean(SETTINGS_LABS_NEW_APP_LAYOUT_KEY, getDefault(R.bool.settings_labs_new_app_layout_default))
}

Expand Down
4 changes: 2 additions & 2 deletions vector/src/main/java/im/vector/app/features/settings/VectorSettingsLabsFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ class VectorSettingsLabsFragment :
}

findPreference<VectorSwitchPreference>(VectorPreferences.SETTINGS_LABS_NEW_APP_LAYOUT_KEY)?.let { pref ->
pref.isVisible = vectorFeatures.isNewAppLayoutEnabled()
pref.isVisible = vectorFeatures.isNewAppLayoutFeatureEnabled()
}

findPreference<VectorSwitchPreference>(VectorPreferences.SETTINGS_LABS_UNREAD_NOTIFICATIONS_AS_TAB)?.let { pref ->
pref.isVisible = !vectorFeatures.isNewAppLayoutEnabled()
pref.isVisible = !vectorFeatures.isNewAppLayoutFeatureEnabled()
Copy link
Member

Choose a reason for hiding this comment

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

same, we want to check the labs flag here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What we should do probably here is make it visible depending on the feature flag, and make it enabled depending on the labs flag

Copy link
Member

Choose a reason for hiding this comment

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

OK, good idea.

}
}

Expand Down
2 changes: 1 addition & 1 deletion vector/src/main/java/im/vector/app/features/settings/VectorSettingsPreferencesFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class VectorSettingsPreferencesFragment :
}

findPreference<Preference>(VectorPreferences.SETTINGS_PREF_SPACE_CATEGORY)!!.let { pref ->
pref.isVisible = !vectorFeatures.isNewAppLayoutEnabled()
pref.isVisible = !vectorFeatures.isNewAppLayoutFeatureEnabled()
Copy link
Member

Choose a reason for hiding this comment

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

same, we want to check the labs flag here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

}

// Url preview
Expand Down
2 changes: 1 addition & 1 deletion vector/src/main/res/xml/vector_settings_labs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@

<im.vector.app.core.preference.VectorSwitchPreference
android:defaultValue="@bool/settings_labs_new_app_layout_default"
android:key="SETTINGS_LABS_ENABLE_NEW_LAYOUT"
android:key="SETTINGS_LABS_NEW_APP_LAYOUT_KEY"
android:summary="@string/labs_enable_new_app_layout_summary"
android:title="@string/labs_enable_new_app_layout_title" />

Expand Down