monal-im / Monal

Monal for XMPP (iOS and macOS)
https://monal-im.org
Other
520 stars 107 forks source link

Integrate "Add Account (advanced)" in WelcomeLogIn.swift #1246

Closed lissine0 closed 4 weeks ago

lissine0 commented 1 month ago
tmolitor-stud-tu commented 1 month ago

I don't like this. the welcome login screen should be easy to use for non-techies and advanced options should be hidden in a separate screen for tech savy people to use.

lissine0 commented 1 month ago

the welcome login screen should be easy to use for non-techies and advanced options should be hidden in a separate screen for tech savy people to use.

That's exactly how it is in this PR. The screenshots above are what's shown when Add Account (advanced) is selected from the settings. Normal Add Account is still the same as before.

What this PR does, is basing Add Account (advanced) on Add Account instead of Account Edit It feels more natural this way.

tmolitor-stud-tu commented 1 month ago

Ah okay, that's okay. But, for the advanced view, could you remove the logo and change the explanatory text at the top to something explaining that this is an advanced expert account creation menu?

tmolitor-stud-tu commented 1 month ago

We should make the direct tls switch only accessible (e.g. enabled) if the hostname or port fields contain something...

lissine0 commented 1 month ago

Ah okay, that's okay. But, for the advanced view, could you remove the logo and change the explanatory text at the top to something explaining that this is an advanced expert account creation menu?

Which one of these looks do you prefer? this:

Or:



We should make the direct tls switch only accessible (e.g. enabled) if the hostname or port fields contain something...

Would you like to also switch the toggle to false if the user types something in hostname / port, then enables directTLS, then clears the hostname / port?

tmolitor-stud-tu commented 1 month ago

I'd prefer the third one. @FriedrichAltheide @matthewrfennell what do you prefer?

Would you like to also switch the toggle to false if the user types something in hostname / port, then enables directTLS, then clears the hostname / port?

I'd say yes.

matthewrfennell commented 1 month ago

I'd prefer the third one. @FriedrichAltheide @matthewrfennell what do you prefer?

I agree

tmolitor-stud-tu commented 1 month ago

okay, third one it is :)

lissine0 commented 1 month ago

I made some other changes aside from those you asked for. Here's a demo of them:

https://github.com/user-attachments/assets/11b97e74-cb73-4d77-8ae4-3484cb669747

The reason why I did this is: the hardcoded port is only used in connection if a hardcoded server is set. And I think the forceDirectTLS setting is also only used if there's a hardcoded server. Thus I made the UI changes to make the users aware that those settings only take effect when setting a hardcoded server.

Also, in the old UI, 5222 is set by default as hardcoded port, even when doing a normal account adding. That can confuse users into thinking that StartTLS is used because of that value. Thus I made 5222 the default only when a hardcoded server is set. Otherwise an empty string is used. (We can store an empty string in a DB column of type Int because of a quirk in SQLite: the column's type is just a hint, and it doesn't prevent us from from storing a string in an int column)

And BTW, you were correct, the old add account (advanced) did indeed disallow plain auth for conversations.im (XMPPEdit.m L342)