Skip to content

feat(wizard): allow to set proxy settings when setting up a new account #8623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
mgallien merged 7 commits into from
Aug 26, 2025
Merged

feat(wizard): allow to set proxy settings when setting up a new account #8623

mgallien merged 7 commits into from
Aug 26, 2025

Conversation

@mgallien
Copy link
Collaborator

@mgallien mgallien commented Aug 20, 2025 •
edited
Loading

close #8602

@mgallien mgallien added feature: ☁️ GUI System tray icon and menu. 2. developing labels Aug 20, 2025
@mgallien mgallien requested a review from kra-mo August 20, 2025 16:43
@mgallien

This comment was marked as outdated.

Sign in to view
@mgallien mgallien force-pushed the feat/proxySettingsNewAccountWizard branch 2 times, most recently from 745181d to 96a907a Compare August 21, 2025 12:03
@kra-mo
Copy link
Member

kra-mo commented Aug 21, 2025

Fair enough, although the button is a bit too inviting for such an obscure setting imo. I'd just place it in the bottom bar the same as Back and Next*, and make it not have a background if that is possible.

* Which by the way really shouldn't have the < and >, at the very least ← and → but it's probably unnecessary anyway.

@mgallien mgallien force-pushed the feat/proxySettingsNewAccountWizard branch from 96a907a to 5629296 Compare August 22, 2025 08:26
@mgallien
Copy link
Collaborator Author

mgallien commented Aug 22, 2025

updated screenshot for the proxy settings dialog
default configuration:
image

system proxy:
image

manual proxy with missing data:
image

once data is filled in, user gets feedback:
image

@mgallien
Copy link
Collaborator Author

mgallien commented Aug 22, 2025 •
edited
Loading

@kra-mo
updated wizard page with better button texts and layout
image

@kra-mo
Copy link
Member

kra-mo commented Aug 22, 2025

@kra-mo

updated wizard page with better button texts and layout

This is nicer, thanks :)

@Rello
Copy link
Collaborator

Rello commented Aug 22, 2025

just an idea: as we are touching it anyway - what about making the window smaller? there is so much wasted space

@mgallien mgallien force-pushed the feat/proxySettingsNewAccountWizard branch 3 times, most recently from 2d037e3 to bf1258f Compare August 25, 2025 09:59
@mgallien mgallien marked this pull request as ready for review August 25, 2025 09:59
@mgallien
Copy link
Collaborator Author

mgallien commented Aug 25, 2025 •
edited
Loading

just an idea: as we are touching it anyway - what about making the window smaller? there is so much wasted space

@Rello the window is that big because it is deigned to keep the same size during all wizard steps
next steps may be full of widgets:

image image

I would also think that this is out of scope for this task

I am advocating a redesign aiming for simplicity
most of the time, setting up a new account and selecting files to sync is way too complicated and does not make it easy for users to sync their existing files (i.e. migrate from local only storage to cloud synced storage)

@mgallien
Copy link
Collaborator Author

mgallien commented Aug 25, 2025

this is now complete and should work (I tested with a local squid instance)
ready for review 😄

nilsding
nilsding requested changes Aug 25, 2025
View reviewed changes
Copy link
Member

@nilsding nilsding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good overall, noticed some minor things though.

We also need to hide the button in the case Theme::doNotUseProxy() returns true.

@mgallien
feat(wizard): allow to set proxy settings when setting up a new account
3fca9be 
Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien
feat(wizard): makes entering proxy settings feasible for new accounts
5373060 
Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien
feat(wizard): improve design of new account wizard
7f599dc 
improved Next and Back button texts

less intrusive proxy settings button

Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien
feat(proxy): store and modify proxy settings for use with new account
c8002bb 
connect the new UI to real proxy settings and allow modification and
store them when the user has finished setting them up

close #8602

Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien
fix: fix typo in dialog title
ac3c391 
Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien
fix: ensure up to date proxy settings dialog when opening it
20e705e 
also ensure we properly check the state of the dialog when needed and only when needed

Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien mgallien force-pushed the feat/proxySettingsNewAccountWizard branch from bf1258f to 20e705e Compare August 26, 2025 09:37
@mgallien mgallien requested a review from nilsding August 26, 2025 09:39
@mgallien mgallien force-pushed the feat/proxySettingsNewAccountWizard branch from d6df74d to a638f9a Compare August 26, 2025 10:24
nilsding
nilsding approved these changes Aug 26, 2025
View reviewed changes
@mgallien mgallien force-pushed the feat/proxySettingsNewAccountWizard branch from a638f9a to f19bf25 Compare August 26, 2025 11:05
@mgallien
fix: ensure proxy settings are visible only when needed
130455e 
when a build is configured without the network proxy feature, ensure we
do not show any UI related to network proxy settings in the new account
wizard

Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien mgallien force-pushed the feat/proxySettingsNewAccountWizard branch from f19bf25 to 130455e Compare August 26, 2025 11:26
@mgallien
Copy link
Collaborator Author

mgallien commented Aug 26, 2025

Looking good overall, noticed some minor things though.

We also need to hide the button in the case Theme::doNotUseProxy() returns true.

@nilsding with the latest push, the proxy settings button should be only visible when the network proxy feature is enabled

@mgallien mgallien enabled auto-merge August 26, 2025 11:28
@github-actions
Copy link

github-actions bot commented Aug 26, 2025

Artifact containing the AppImage: nextcloud-appimage-pr-8623.zip

SHA256 checksum: 976180afaa9b087fc03d09bd16ddd4d379b8d268f693eeb1eafef1cb9974ac9b

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 26, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
Image 0.0% Coverage on New Code (required ≥ 80%)
Image 39 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Image Catch issues before they fail your Quality Gate with our IDE extension Image SonarQube for IDE

@mgallien mgallien merged commit e0a2217 into master Aug 26, 2025
17 of 19 checks passed
@mgallien mgallien deleted the feat/proxySettingsNewAccountWizard branch August 26, 2025 12:03
@mgallien
Copy link
Collaborator Author

mgallien commented Aug 26, 2025

/backport to stable-3.17

@backportbot backportbot bot mentioned this pull request Aug 26, 2025
Merged
@smcgu smcgu mentioned this pull request Sep 17, 2025
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nilsding nilsding nilsding approved these changes

@kra-mo kra-mo Awaiting requested review from kra-mo

@camilasan camilasan Awaiting requested review from camilasan camilasan is a code owner

@i2h3 i2h3 Awaiting requested review from i2h3 i2h3 is a code owner

Assignees

No one assigned

Labels

2. developing feature: ☁️ GUI System tray icon and menu.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: 3.17.0 Proxy Handling Broken

5 participants

@mgallien @kra-mo @Rello @nilsding