Conversation
There was a problem hiding this comment.
Pull request overview
Adds an authorization (login) configuration UI to the PyQt GUI client, wiring it through to the exposehost.client.Client auth proxy feature.
Changes:
- Added an
AuthWidgetUI section to enable/disable authorization and manage username/password pairs. - Passed auth settings from the GUI into
Client(..., auth_enabled=..., auth_users=...), and validated that at least one credential pair is provided when enabled. - Improved Home page layout with a scroll area and made SVG logo loading optional when
PyQt5.QtSvgisn’t available.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -664,15 +827,28 @@ def start_client(self): | |||
| self.port_input.setFocus() | |||
| return | |||
|
|
|||
There was a problem hiding this comment.
The subdomain validation check appears to be missing: the warning at lines 830–832 is currently unreachable (it sits after the port validation return and is not guarded by an if not subdomain:). Add the if not subdomain: condition back so subdomain is actually required before starting the client.
| if not subdomain: |
| self.client = Client( | ||
| host, port, "exposehost.me", 1435, protocol, subdomain, | ||
| auth_enabled=auth_config['enabled'], | ||
| auth_users=auth_config['users'] | ||
| ) |
There was a problem hiding this comment.
port is kept as a string and passed into Client(...). The Client later uses this value in asyncio.open_connection(..., self.port), which expects an int port (the CLI casts it to int(port)). Convert the GUI port to an integer before constructing Client to avoid runtime TypeError when connecting.
| class AuthWidget(QWidget): | ||
| def __init__(self, parent=None): | ||
| super().__init__(parent) | ||
| self.users = [] # List of tuples (user_input, pass_input, row_layout, delete_btn) |
There was a problem hiding this comment.
AuthWidget.__init__ initializes self.users, but this list is never read from or appended to (rows are tracked via the layout/widgets instead). Consider removing self.users to avoid misleading state, or update the implementation to keep it in sync and use it in get_auth_config().
| self.users = [] # List of tuples (user_input, pass_input, row_layout, delete_btn) |
| # Don't remove the last row if it's the only one? | ||
| # Actually, let's allow removing all, but maybe auto-add one if empty when enabling? | ||
| # For now simple remove | ||
| if self.user_list_layout.count() <= 1: | ||
| # Maybe clear instead of remove if it's the last one? | ||
| # Or just allow removing. | ||
| pass | ||
|
|
There was a problem hiding this comment.
remove_user_row contains an if self.user_list_layout.count() <= 1: block that only passes, but the row is removed unconditionally afterward. Either remove this dead block, or implement the intended behavior (e.g., prevent deleting the last row, or clear fields instead) to match the comment and avoid confusion.
| # Don't remove the last row if it's the only one? | |
| # Actually, let's allow removing all, but maybe auto-add one if empty when enabling? | |
| # For now simple remove | |
| if self.user_list_layout.count() <= 1: | |
| # Maybe clear instead of remove if it's the last one? | |
| # Or just allow removing. | |
| pass | |
| # For now, always remove the specified row widget |
| content_layout.addWidget(services_label) | ||
|
|
||
| self.service_list = QListWidget() | ||
| self.service_list.setFixedHeight(180) # Made it biggervertically |
There was a problem hiding this comment.
Typo in the inline comment: “biggervertically” is missing a space (e.g., “bigger vertically”).
| self.service_list.setFixedHeight(180) # Made it biggervertically | |
| self.service_list.setFixedHeight(180) # Made it bigger vertically |
No description provided.