-
Notifications
You must be signed in to change notification settings - Fork 768
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
Changes from 11 commits
2450954
fbe5918
a851b0a
5e0d84b
91b4918
468c7b6
31a3552
b7efd63
d77ce27
9564c8f
6c23634
afbb76f
64c8789
37b2163
6badbe7
2ab0343
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Adds New App Layout into Labs |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,7 +249,7 @@ abstract class VectorBaseActivity<VB : ViewBinding> : AppCompatActivity(), Maver | |
|
||
initUiAndData() | ||
|
||
if (vectorFeatures.isNewAppLayoutEnabled()) { | ||
if (vectorPreferences.isNewAppLayoutEnabled()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since both Something like 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
|
||
|
@@ -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 | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
fun isNewAppLayoutFeatureEnabled(): Boolean | ||
fun isNewDeviceManagementEnabled(): Boolean | ||
} | ||
|
||
|
@@ -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 |
---|---|---|
|
@@ -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) { | ||
|
@@ -217,12 +217,13 @@ class HomeActivity : | |
roomListSharedActionViewModel = viewModelProvider[RoomListSharedActionViewModel::class.java] | ||
views.drawerLayout.addDrawerListener(drawerListener) | ||
if (isFirstCreation()) { | ||
if (vectorFeatures.isNewAppLayoutEnabled()) { | ||
if (vectorPreferences.isNewAppLayoutEnabled()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
} | ||
} | ||
|
||
|
@@ -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() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,8 +75,22 @@ class VectorSettingsLabsFragment : | |
} | ||
} | ||
|
||
findPreference<VectorSwitchPreference>(VectorPreferences.SETTINGS_LABS_UNREAD_NOTIFICATIONS_AS_TAB)!!.let { | ||
it.isVisible = !vectorFeatures.isNewAppLayoutEnabled() | ||
findPreference<VectorSwitchPreference>(VectorPreferences.SETTINGS_LABS_NEW_APP_LAYOUT_KEY)?.let { pref -> | ||
pref.isVisible = vectorFeatures.isNewAppLayoutFeatureEnabled() | ||
|
||
pref.onPreferenceClickListener = Preference.OnPreferenceClickListener { | ||
onNewLayoutPreferenceClicked() | ||
true | ||
} | ||
} | ||
|
||
configureUnreadNotificationsAsTabPreference() | ||
} | ||
|
||
private fun configureUnreadNotificationsAsTabPreference() { | ||
findPreference<VectorSwitchPreference>(VectorPreferences.SETTINGS_LABS_UNREAD_NOTIFICATIONS_AS_TAB)?.let { pref -> | ||
pref.isVisible = !vectorFeatures.isNewAppLayoutFeatureEnabled() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same, we want to check the labs flag here. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, good idea. |
||
pref.isEnabled = !vectorPreferences.isNewAppLayoutEnabled() | ||
} | ||
} | ||
|
||
|
@@ -119,4 +133,11 @@ class VectorSettingsLabsFragment : | |
displayLoadingView() | ||
MainActivity.restartApp(requireActivity(), MainActivityArgs(clearCache = true)) | ||
} | ||
|
||
/** | ||
* Action when new layout preference switch is actually clicked. | ||
*/ | ||
private fun onNewLayoutPreferenceClicked() { | ||
configureUnreadNotificationsAsTabPreference() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,7 +102,8 @@ class VectorSettingsPreferencesFragment : | |
} | ||
|
||
findPreference<Preference>(VectorPreferences.SETTINGS_PREF_SPACE_CATEGORY)!!.let { pref -> | ||
pref.isVisible = !vectorFeatures.isNewAppLayoutEnabled() | ||
pref.isVisible = !vectorFeatures.isNewAppLayoutFeatureEnabled() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same, we want to check the labs flag here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above |
||
pref.isEnabled = !vectorPreferences.isNewAppLayoutEnabled() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to be refreshed as well (like in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry, please ignore, this is not the same Fragment. |
||
} | ||
|
||
// Url preview | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, good idea! Thanks