feat: add proxy page #78
Conversation
aanorbel
commented
Aug 21, 2024
aanorbel
commented
| . | . |
|---|---|
 |
 |
| if (state.proxyType != ProxyType.CUSTOM || | ||
| (isValidDomainNameOrIp(state.proxyHost ?: "") && isValidPort(state.proxyPort ?: 0)) | ||
| ) { | ||
| onEvent(ProxyViewModel.Event.BackClicked) |
There was a problem hiding this comment.
It feels weird to prevent the back button from working, without any error message.
| OutlinedTextField( | ||
| value = state.proxyProtocol.toCustomProtocol(), | ||
| onValueChange = { }, | ||
| label = { Text(stringResource(Res.string.Settings_Proxy_Custom_Protocol), maxLines = 1) }, | ||
| modifier = Modifier.weight(0.3f), | ||
| readOnly = true, | ||
| singleLine = true, | ||
| maxLines = 1, | ||
| trailingIcon = { | ||
| IconButton(onClick = { expanded = true }) { | ||
| Icon( | ||
| Icons.Filled.ArrowDropDown, | ||
| contentDescription = stringResource(Res.string.Settings_Proxy_Custom_Protocol), | ||
| ) | ||
| } | ||
| }, | ||
| ) | ||
| DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { | ||
| protocols.forEach { protocol -> | ||
| DropdownMenuItem(text = { Text(protocol.protocol) }, onClick = { | ||
| expanded = false | ||
| onEvent( | ||
| ProxyViewModel.Event.ProtocolChanged( | ||
| protocol = protocol, | ||
| ), | ||
| ) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
This isn't working very well. I can tap the contents of the Custom Proxy field and it goes into a focused state, but doesn't open the dropdown. Probably the whole field needs to open the dropdown on touch/focus.
| value = hostname, | ||
| onValueChange = { | ||
| hostname = it | ||
| if (isValidDomainNameOrIp(it)) { |
There was a problem hiding this comment.
I feel validations should happen on the ViewModel, instead of the View.
| enum class ProxyType(val label: StringResource, val value: String) { | ||
| NONE( | ||
| label = Res.string.Settings_Proxy_None, | ||
| value = "none", | ||
| ), | ||
| PSIPHON( | ||
| label = Res.string.Settings_Proxy_Psiphon, | ||
| value = "psiphon", | ||
| ), | ||
| CUSTOM(label = Res.string.Settings_Proxy_Custom, value = "custom"), | ||
| } | ||
|
|
||
| enum class ProxyProtocol(val protocol: String) { | ||
| NONE("none"), | ||
| HTTP("http"), | ||
| HTTPS("https"), | ||
| SOCKS5("socks5"), | ||
| PSIPHON("psiphon"), | ||
| ; | ||
|
|
||
| fun proxyType(): ProxyType { | ||
| return when (this) { | ||
| NONE -> ProxyType.NONE | ||
| PSIPHON -> ProxyType.PSIPHON | ||
| else -> ProxyType.CUSTOM | ||
| } | ||
| } | ||
|
|
||
| fun toCustomProtocol(): String { | ||
| return when (this) { | ||
| PSIPHON, NONE -> "" | ||
| else -> this.protocol | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This models should be in the data/models folder.
| suspend fun getProxyString(): String { | ||
| val proxyHost = | ||
| getValueByKey(SettingsKey.PROXY_HOSTNAME).firstOrNull() as String? ?: return "" | ||
| val proxyPort = getValueByKey(SettingsKey.PROXY_PORT).firstOrNull() as Int? ?: return "" | ||
|
|
||
| when ( | ||
| val proxyProtocol = | ||
| getValueByKey(SettingsKey.PROXY_PROTOCOL).firstOrNull() as String? | ||
| ) { | ||
| ProxyProtocol.NONE.protocol -> return "" | ||
| ProxyProtocol.PSIPHON.protocol -> return "psiphon://" | ||
| ProxyProtocol.SOCKS5.protocol, ProxyProtocol.HTTP.protocol, ProxyProtocol.HTTPS.protocol -> { | ||
| val formattedHost = if (isIPv6(proxyHost)) { | ||
| "[$proxyHost]" | ||
| } else { | ||
| proxyHost | ||
| } | ||
| return "$proxyProtocol://$formattedHost:$proxyPort/" | ||
| } | ||
|
|
||
| else -> return "" | ||
| } | ||
| } |
There was a problem hiding this comment.
I feel all of this is more than the PreferencesRepository should know about the Proxy logic. At most it could receive a ProxySettings object and know how to split that into individual preferences, and the other way around.
But generating a proxy string could happen inside a ProxySettings object. The validation could happen on ProxySettings, in the ViewModel, or the domain.
| } | ||
| } | ||
|
|
||
| suspend fun getProxyString(): String { |
There was a problem hiding this comment.
Now we can integrate this proxy string into the Engine:
| var port by rememberSaveable { mutableStateOf(state.proxyPort?.toString() ?: "") } | ||
| val protocols = | ||
| listOf(ProxyProtocol.HTTP, ProxyProtocol.HTTPS, ProxyProtocol.SOCKS5) | ||
| OutlinedTextField( |
There was a problem hiding this comment.
Tapping on this field still doesn't open the dropdown menu. You need to tap on the dropdown arrow.
| * ProxySettings contains the settings configured inside the proxy. Please, see the | ||
| * documentation of proxy activity for the design rationale. | ||
| */ | ||
| class ProxySettings { |
There was a problem hiding this comment.
This class is not being used.
| * Creates a new ProxySettings object from the values stored in the preference repository. | ||
| * @param preferenceRepository the [PreferenceRepository]. | ||
| */ | ||
| suspend fun newProxySettings(preferenceRepository: PreferenceRepository): ProxySettings { |
There was a problem hiding this comment.
It's not a good practice to use a dependency like PreferenceRepository inside a data model. The data model should only receive the values.
2 participants
