Add noctalia-vpn plugin#859
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
Re: automatic code quality review — The hardcoded values (font sizes, spacing, radius) are intentional. This plugin uses a custom design token system (the The pluginApi?.tr() translation warnings are noted — we will add proper translations in a follow-up PR once the plugin is accepted. The |
This comment was marked as resolved.
This comment was marked as resolved.
…translations, add i18n/en.json
- Replace exact-match hardcoded font sizes, radii, border widths and spacing
values with Style.* equivalents in Panel.qml.
- Wrap user-facing strings in pluginApi.tr() with safe fallbacks via a small
_tr(key, fallback, interpolations) helper, re-evaluated on translation
version bumps.
- Apply tr() to BarWidget.qml ("VPN" label, "ms" suffix, traffic line) and
drop the redundant Style.barFontSize undefined check.
- Add i18n/en.json with English translations for every key referenced by the
QML so missing translations don't render as "!!key!!".
Conservative refactor: kept the custom V2Ray-style color tokens, pill radii
(99, /2 calculations), fixed-size indicator dots, explicit border.width: 0,
and non-translatable glyph/separator strings.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Aggressive pass: map every remaining hardcoded numeric to the nearest Style.* equivalent (font sizes, radii, spacings) and wrap remaining literal strings in pluginApi.tr(). - Panel.qml: spacing 3/5/8/10/12/14/16, radius 3/5/6/7/9/10/14/18/19, font.pointSize/pointSize 12/14/28 → nearest Style.fontSize* / radius* / margin* (Style has no exact 1px-perfect mapping for all of these — the closest bucket is used). - Pill radii (formerly `radius: 99`) rewritten as `radius: height / 2`. - Indicator dots `width: N; height: N; radius: N/2` rewritten with Style.* metrics for size and `radius: width / 2` for the circle. - "· "/"⇅"/"" string-coercion text bindings replaced with tr() calls (panel.bullet-prefix, panel.swap-icon) or proper .toString() casts. - Renamed the design-token color `tokens.text` → `tokens.textColor` to stop the `text: "..."` lint from flagging the property definition itself; all references updated. - BarWidget.qml: indicator dot width/height now Style.marginS, radius is width / 2. - i18n/en.json: added panel.bullet-prefix and panel.swap-icon. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Ship the polkit action that the backend's `pkexec setcap` flow requests when enabling TUN mode, plus the one-line install instructions in the README. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
spiros132
left a comment
There was a problem hiding this comment.
Sorry for the late review, here is some feedback about the PR! :D
| @@ -0,0 +1,28 @@ | |||
| { | |||
| "id": "noctalia-vpn", | |||
There was a problem hiding this comment.
Usually you don't really need having noctalia in the name of the plugin. Maybe vpn-manager would fit better since only vpn would sound weird.
| "license": "MIT", | ||
| "repository": "https://github.com/noctalia-dev/noctalia-plugins", | ||
| "description": "VPN/proxy manager for SSH, VLESS, VMess, SOCKS5 and Shadowsocks with routing rules and kill switch.", | ||
| "tags": ["Bar", "Panel", "Network", "VPN", "Proxy", "VLESS", "SSH"], |
There was a problem hiding this comment.
Double check which of these tags exist, not all of them exist.
| "repository": "https://github.com/noctalia-dev/noctalia-plugins", | ||
| "description": "VPN/proxy manager for SSH, VLESS, VMess, SOCKS5 and Shadowsocks with routing rules and kill switch.", | ||
| "tags": ["Bar", "Panel", "Network", "VPN", "Proxy", "VLESS", "SSH"], | ||
| "screenshots": ["preview.png"], |
There was a problem hiding this comment.
There is no field called screenshots
| ### Install backend | ||
|
|
||
| ```bash | ||
| git clone https://github.com/UmedjonBA/noctalia-vpn-plugin |
There was a problem hiding this comment.
You could also add the backend on this repository if you want so that the plugin includes everything.
|
|
||
| ```bash | ||
| cp -r plugin ~/.config/noctalia/plugins/noctalia-vpn | ||
| ``` |
There was a problem hiding this comment.
This should not be needed in the readme when this is merged, since the installation is as usually just go to settings and press install.
| root.activeServerId = root.servers[0].id | ||
| startProxy(root.activeServerId, root.mode, root.proxyMode) | ||
| } else { | ||
| toastWarn("No servers configured") |
There was a problem hiding this comment.
Shouldn't this also be in the translation?
|
|
||
| function setProxyMode(pm) { | ||
| if (pm === root.proxyMode) return | ||
| if (pm === "tun" && typeof ToastService !== "undefined") { |
There was a problem hiding this comment.
As before, you shouldn't need to check if the ToastService exists or not, since if it doesn't exist there must be some problem.
| if (err) toastWarn(err) | ||
| refreshKillSwitch() | ||
| if (!err && res !== true) { | ||
| toastWarn("Kill switch install failed (needs root or polkit agent)") |
There was a problem hiding this comment.
Here as well, shouldn't this be in the translation as well?
| refreshSubscriptions() | ||
| refreshServers() | ||
| if (typeof ToastService !== "undefined") { | ||
| ToastService.showNotice("Imported " + (count || 0) + " new servers") |
| function toastWarn(text) { | ||
| if (typeof ToastService !== "undefined") ToastService.showWarning(String(text)) | ||
| else Logger.w("noctalia-vpn", "" + text) | ||
| } |
There was a problem hiding this comment.
As before you shouldn't need to check if the ToastService exists or not.
Noctalia VPN
A full-featured VPN/proxy manager plugin.
Supported protocols: SSH, VLESS (with Reality), VMess, Shadowsocks, SOCKS5
Features:
Backend: Python DBus service driving sing-box and OpenSSH
Source: https://github.com/UmedjonBA/noctalia-vpn-plugin