Skip to content

Commit 49d8dcd

Browse files
nbradburyclaude
nbradbury
and
claude
authored
Fix issues introduced by login lib removal (#22703)
* Fix issues introduced by login lib removal - Use single root layout with loading overlay in LoginActivity to avoid multiple setContentView() calls that orphan fragments - Wrap unbindService() in try-catch for IllegalArgumentException - Reset cached mLoginFlow in onNewIntent() to prevent stale values - Remove custom get() on loadingStateFlow to avoid recreating wrapper - Remove redundant e.printStackTrace() already logged via appLogWrapper - Require dot in site URL validation to reject single-word inputs - Make loading dialog dismissible by adding cancel discovery callback Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add hideLoadingOverlay() to LoginActivity Adds the inverse of showLoadingOverlay() so future code paths can transition back from the loading state to the fragment UI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cc26b8d commit 49d8dcd

File tree

6 files changed

+84
-22
lines changed

6 files changed

+84
-22
lines changed

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/LoginActivity.java‎

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import android.content.Intent;
77
import android.net.Uri;
88
import android.os.Bundle;
9+
import android.view.LayoutInflater;
910
import android.view.View;
1011
import android.view.ViewGroup;
1112
import android.widget.FrameLayout;
@@ -88,6 +89,7 @@ public class LoginActivity extends BaseAppCompatActivity implements
8889
private static final String KEY_SHARE_FLOW_LOGIN_LAUNCHED = "KEY_SHARE_FLOW_LOGIN_LAUNCHED";
8990

9091
private int mFragmentContainerId;
92+
private View mLoadingOverlay;
9193

9294
/**
9395
* Check if there's a pending share flow. Used by
@@ -156,15 +158,27 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
156158

157159
LoginFlowThemeHelper.injectMissingCustomAttributes(getTheme());
158160

161+
ViewGroup.LayoutParams matchParent = new ViewGroup.LayoutParams(
162+
ViewGroup.LayoutParams.MATCH_PARENT,
163+
ViewGroup.LayoutParams.MATCH_PARENT
164+
);
165+
166+
FrameLayout rootContainer = new FrameLayout(this);
167+
rootContainer.setLayoutParams(matchParent);
168+
159169
FrameLayout fragmentContainer = new FrameLayout(this);
160170
mFragmentContainerId = R.id.fragment_container;
161171
fragmentContainer.setId(mFragmentContainerId);
162172
fragmentContainer.setFitsSystemWindows(false);
163-
fragmentContainer.setLayoutParams(new ViewGroup.LayoutParams(
164-
ViewGroup.LayoutParams.MATCH_PARENT,
165-
ViewGroup.LayoutParams.MATCH_PARENT
166-
));
167-
setContentView(fragmentContainer);
173+
fragmentContainer.setLayoutParams(matchParent);
174+
rootContainer.addView(fragmentContainer);
175+
176+
mLoadingOverlay = LayoutInflater.from(this)
177+
.inflate(R.layout.login_loading, rootContainer, false);
178+
mLoadingOverlay.setVisibility(View.GONE);
179+
rootContainer.addView(mLoadingOverlay);
180+
181+
setContentView(rootContainer);
168182

169183
if (savedInstanceState == null) {
170184
mLoginAnalyticsListener.trackLoginAccessed();
@@ -179,7 +193,10 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
179193
showWPcomLoginScreen(this);
180194
break;
181195
case SELF_HOSTED:
182-
showFragment(new LoginSiteApplicationPasswordFragment(), LoginSiteApplicationPasswordFragment.TAG);
196+
showFragment(
197+
new LoginSiteApplicationPasswordFragment(),
198+
LoginSiteApplicationPasswordFragment.TAG
199+
);
183200
break;
184201
}
185202
} else {
@@ -194,9 +211,9 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
194211
}
195212

196213
// If we're waiting for sites (e.g. after config change during post-OAuth fetch),
197-
// show the loading screen instead of the fragment container
214+
// show the loading overlay instead of the fragment container
198215
if (mIsWaitingForSitesToLoad) {
199-
setContentView(R.layout.login_loading);
216+
showLoadingOverlay();
200217
}
201218

202219
initViewModel();
@@ -233,12 +250,17 @@ public void onSaveInstanceState(@NonNull Bundle outState) {
233250
protected void onNewIntent(@NonNull Intent intent) {
234251
super.onNewIntent(intent);
235252
setIntent(intent);
253+
mLoginFlow = null;
236254

237255
// Handle OAuth callback when activity is reused (singleTop)
238256
String dataString = intent.getDataString();
239257
if (mLoginHelper.hasOAuthCallback(dataString)) {
240258
intent.setData(null);
241-
setContentView(R.layout.login_loading);
259+
if (mLoadingOverlay != null) {
260+
showLoadingOverlay();
261+
} else {
262+
setContentView(R.layout.login_loading);
263+
}
242264
mLoginHelper.tryLoginWithDataString(
243265
dataString,
244266
() -> loggedInAndFinish(new ArrayList<>(), true),
@@ -350,6 +372,26 @@ private void showPrologueScreen() {
350372
);
351373
}
352374

375+
private void showLoadingOverlay() {
376+
if (mLoadingOverlay != null) {
377+
mLoadingOverlay.setVisibility(View.VISIBLE);
378+
}
379+
View fragmentContainer = findViewById(mFragmentContainerId);
380+
if (fragmentContainer != null) {
381+
fragmentContainer.setVisibility(View.GONE);
382+
}
383+
}
384+
385+
private void hideLoadingOverlay() {
386+
if (mLoadingOverlay != null) {
387+
mLoadingOverlay.setVisibility(View.GONE);
388+
}
389+
View fragmentContainer = findViewById(mFragmentContainerId);
390+
if (fragmentContainer != null) {
391+
fragmentContainer.setVisibility(View.VISIBLE);
392+
}
393+
}
394+
353395
private void showLoginError(@NonNull Exception error) {
354396
AppLog.e(T.MAIN, "OAuth login failed", error);
355397

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/WPcomLoginHelper.kt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ class ServiceConnection(
139139
}
140140

141141
fun unbind() {
142-
boundContext?.unbindService(this)
142+
try {
143+
boundContext?.unbindService(this)
144+
} catch (_: IllegalArgumentException) {
145+
// Already unbound or never bound
146+
}
143147
boundContext = null
144148
client = null
145149
session = null

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/LoginSiteApplicationPasswordFragment.kt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ class LoginSiteApplicationPasswordFragment : Fragment() {
6262

6363
AppThemeM3 {
6464
LoginSiteApplicationPasswordScreen(
65-
isLoading = isLoading,
6665
errorMessage = errorMessage,
6766
onBackClick = {
6867
requireActivity().onBackPressedDispatcher.onBackPressed()
@@ -76,6 +75,11 @@ class LoginSiteApplicationPasswordFragment : Fragment() {
7675
},
7776
onErrorDismissed = {
7877
viewModel.clearError()
78+
},
79+
onCancelLoading = if (isLoading) {
80+
{ viewModel.cancelDiscovery() }
81+
} else {
82+
null
7983
}
8084
)
8185
}

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/LoginSiteApplicationPasswordScreen.kt‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,22 +49,25 @@ import org.wordpress.android.R
4949
@OptIn(ExperimentalMaterial3Api::class)
5050
@Composable
5151
fun LoginSiteApplicationPasswordScreen(
52-
isLoading: Boolean,
5352
errorMessage: String?,
5453
onBackClick: () -> Unit,
5554
onHelpClick: (cleanedAddress: String) -> Unit,
5655
onContinueClick: (cleanedAddress: String) -> Unit,
5756
onErrorDismissed: () -> Unit,
58-
modifier: Modifier = Modifier
57+
modifier: Modifier = Modifier,
58+
onCancelLoading: (() -> Unit)? = null
5959
) {
6060
var siteAddress by rememberSaveable { mutableStateOf("") }
6161
val focusRequester = remember { FocusRequester() }
62+
val isLoading = onCancelLoading != null
6263

6364
val cleanedAddress = siteAddress.trim().replace(Regex("[\r\n]"), "")
64-
val isValid = cleanedAddress.isNotEmpty() && Patterns.WEB_URL.matcher(cleanedAddress).matches()
65+
val isValid = cleanedAddress.isNotEmpty()
66+
&& cleanedAddress.contains(".")
67+
&& Patterns.WEB_URL.matcher(cleanedAddress).matches()
6568

66-
if (isLoading) {
67-
LoadingDialog()
69+
if (onCancelLoading != null) {
70+
LoadingDialog(onDismiss = onCancelLoading)
6871
}
6972

7073
Scaffold(
@@ -157,8 +160,8 @@ fun LoginSiteApplicationPasswordScreen(
157160
}
158161

159162
@Composable
160-
private fun LoadingDialog() {
161-
Dialog(onDismissRequest = { }) {
163+
private fun LoadingDialog(onDismiss: () -> Unit) {
164+
Dialog(onDismissRequest = onDismiss) {
162165
Surface(
163166
shape = MaterialTheme.shapes.medium,
164167
tonalElevation = 8.dp

‎WordPress/src/main/java/org/wordpress/android/ui/accounts/login/applicationpassword/LoginSiteApplicationPasswordViewModel.kt‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package org.wordpress.android.ui.accounts.login.applicationpassword
33
import androidx.lifecycle.ViewModel
44
import androidx.lifecycle.viewModelScope
55
import dagger.hilt.android.lifecycle.HiltViewModel
6+
import kotlinx.coroutines.Job
67
import kotlinx.coroutines.channels.Channel
78
import kotlinx.coroutines.flow.MutableStateFlow
89
import kotlinx.coroutines.flow.asStateFlow
@@ -19,21 +20,30 @@ class LoginSiteApplicationPasswordViewModel @Inject constructor(
1920
val discoveryURL = _discoveryURL.receiveAsFlow()
2021

2122
private val _loadingStateFlow = MutableStateFlow(false)
22-
val loadingStateFlow get() = _loadingStateFlow.asStateFlow()
23+
val loadingStateFlow = _loadingStateFlow.asStateFlow()
2324

2425
private val _errorMessage = MutableStateFlow<String?>(null)
2526
val errorMessage = _errorMessage.asStateFlow()
2627

28+
private var discoveryJob: Job? = null
29+
2730
fun runApiDiscovery(siteUrl: String) {
2831
_errorMessage.value = null
2932
_loadingStateFlow.value = true
30-
viewModelScope.launch {
31-
val discoveryUrl = applicationPasswordLoginHelper.getAuthorizationUrlComplete(siteUrl)
33+
discoveryJob = viewModelScope.launch {
34+
val discoveryUrl = applicationPasswordLoginHelper
35+
.getAuthorizationUrlComplete(siteUrl)
3236
_discoveryURL.send(discoveryUrl)
3337
_loadingStateFlow.value = false
3438
}
3539
}
3640

41+
fun cancelDiscovery() {
42+
discoveryJob?.cancel()
43+
discoveryJob = null
44+
_loadingStateFlow.value = false
45+
}
46+
3747
fun setError(message: String) {
3848
_errorMessage.value = message
3949
}

‎WordPress/src/main/java/org/wordpress/android/ui/mysite/cards/applicationpassword/ApplicationPasswordViewModelSlice.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ class ApplicationPasswordViewModelSlice @Inject constructor(
134134
AppLog.T.MAIN,
135135
"A_P: Exception validating credentials for ${site.url}: ${e::class.simpleName}: ${e.message}"
136136
)
137-
e.printStackTrace()
138137
uiModelMutable.postValue(null)
139138
}
140139
}

0 commit comments

Comments
 (0)
⚡