android icon indicating copy to clipboard operation
android copied to clipboard

Hide 'Wait' if WebViewActivity error is not due to external bus timeout

Open jpelgrom opened this issue 1 year ago • 2 comments

Summary

Hide the 'Wait' button in WebViewActivity errors if the error is not due to an external bus timeout, but a more general loading error where there's nothing to wait for. The PR also slightly updates what the refresh button does to ensure loading errors on refresh trigger immediately (instead of waiting for a timeout) + uses the URL it says on the button text.

This PR was inspired by the (previously unheard) confusion here, and looking back perhaps also by this suggested change in there (although that change wasn't explained, and incomplete).

Screenshots

You get a smaller dialog without a wait button:

Light Dark
Companion app with 'Unable to connect to Home Assistant' error, and only two buttons, which can now be displayed on one line instead of three, light mode Companion app with 'Unable to connect to Home Assistant' error, and only two buttons, which can now be displayed on one line instead of three, dark mode

Link to pull request in Documentation repository

n/a

Any other notes

jpelgrom avatar May 17 '24 17:05 jpelgrom

I also started with this simple fix. It doesn't fix ALL problems. The SETTINGS button needs to be fixed too.

I hid WAIT. It's horrible. Android moves buttons. You see 3 buttons. After 10 seconds you see 2 buttons in a different order. A missed click is guaranteed.

Accept my PR. I've already walked this path.

Roffild avatar May 18 '24 07:05 Roffild

After 10 seconds again:

bug

Roffild avatar May 18 '24 07:05 Roffild

I also started with this simple fix. It doesn't fix ALL problems.

in order to fix things iteratively we need to go step by step, we won't be able to fix all issues in a single PR but we can begin to walk through the steps and determine what's appropriate

The SETTINGS button needs to be fixed too.

we should not attempt to fix multiple things here and also users do indeed need to access Settings so not sure why you say it needs to be fixed, its doing what its intended to do, take users to settings to adjust things like their URL.

Accept my PR.

As mentioned in the previous PR please do not demand that PRs get merged and accepted, there is a process to follow and your PR still has lots of open comments that you did not address.

dshokouhi avatar Jun 04 '24 16:06 dshokouhi

Why hide the button? To annoy the user?

The pop-up does not update automatically:

// Line 1287:
if (isShowingError || !isStarted || isRelaunching) {
    return
}
isShowingError = true

"WAIT" is hidden only from WebViewPresenterImpl.onGetExternalAuth(). waitForConnection() is almost always called earlier:

// Line 1414:
if (url != null) {
    loadUrl(url = url, keepHistory = true, openInApp = true) // <= waitForConnection :D
} else {
    waitForConnection()
}
////////////////
private fun waitForConnection() {
           showError(errorType = ErrorType.TIMEOUT_EXTERNAL_BUS) // <=
}

This patch does not work!

Roffild avatar Jun 07 '24 17:06 Roffild

I am a professional programmer. I really need a working application. You don't want or can't reproduce all the error variants from the server.

I tested all the variants. I looked at the Android source code. I would write UnitTests, but there are no tests. (Why?!)

Maybe you can trust me?

Roffild avatar Jun 07 '24 18:06 Roffild

Sorry, but the only one that seems to be confused by the app's behavior or having issues with it to this extent is you.

Merging as this change was approved by the other 2 main contributors.

jpelgrom avatar Jun 14 '24 21:06 jpelgrom

@JBassett, have you actually reproduced the problem? Or just approved the patch?

This patch does not work when WebView has cache. You also cannot reproduce bug #4439 without a cache.

This patch fixes external connection checking. My #4358 fixes the internal status of the WebView.

Roffild avatar Jun 15 '24 15:06 Roffild

Conflict in UI/UX: Most users do not understand the difference between REFRESH and WAIT. I don't understand the difference either (without source code).

If you want to reduce the size of the Pop-up, then combine the buttons. I want to see a fixed size Pop-up!

As users, do you really like: After start 3 buttons, after 180 seconds 2 buttons in a different order, after 10 again 3 buttons... Really?

Roffild avatar Jun 15 '24 15:06 Roffild