Skip to content
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.

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

wireguard: move enable checkbox to apply button and remove the general form #7223

Closed
2 tasks done
fichtner opened this issue Feb 9, 2024 · 1 comment
Closed
2 tasks done
Assignees
Labels
feature Adding new functionality roadmap Major roadmap item
Milestone

Comments

@fichtner
Copy link
Member

fichtner commented Feb 9, 2024

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

Is your feature request related to a problem? Please describe.

General tab is a bit low on value and just now with #6973 we have a nice way to ensure people find the "enable" checkbox by placing it with the apply button.

Describe the solution you like

In a nutshell remove the tab, maybe rename the Settings to Configuration to align with others (or add the individual tabs into the menu like we do with the firmware menu). Make sure the docs reflect the change but it shouldn't cause the amount of friction of earlier more fundamental reworks.

Describe alternatives you considered

Adding more options to the general settings tab but there aren't any that make sense. ;)

Additional context

Loosely related to #6973 as mentioned.

@fichtner fichtner added the feature Adding new functionality label Feb 9, 2024
@fichtner fichtner added this to the 24.7 milestone Feb 9, 2024
@fichtner fichtner self-assigned this Feb 9, 2024
@fichtner
Copy link
Member Author

One note from @swhite2 was the possibility of making the enable/apply pattern a reusable hook of some sort.

Also the services hook is not there. It's difficult anyway to display per instance. Just need to check status page and services page more or less align with the whole implementation.

fichtner added a commit that referenced this issue Feb 21, 2024
Since @swhite2 was asking to make a generalized pattern I played
with the layout a bit and ended up just moving the form below so
it actually holds more settings if it has.  This way the base_form
could be extended to only show a subset of settings (like enable)
but also show the full set if more settings are coming in.

While here adjust the menu structure and remove the ordering of
the VPN types since they order naturally.
fichtner added a commit that referenced this issue Mar 12, 2024
Since @swhite2 was asking to make a generalized pattern I played
with the layout a bit and ended up just moving the form below so
it actually holds more settings if it has.  This way the base_form
could be extended to only show a subset of settings (like enable)
but also show the full set if more settings are coming in.

While here adjust the menu structure and remove the ordering of
the VPN types since they order naturally.
fichtner added a commit that referenced this issue Apr 3, 2024
(cherry picked from commit 935f041)
(cherry picked from commit bc2ca23)
(cherry picked from commit 27c66f6)
(cherry picked from commit 13b685a)
(cherry picked from commit e53da1f)
(cherry picked from commit 768d900)
(cherry picked from commit 550dacf)
@fichtner fichtner added roadmap Major roadmap item and removed roadmap Major roadmap item labels Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding new functionality roadmap Major roadmap item
Development

No branches or pull requests

1 participant