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

Allow selecting region when restoring default bands list #728

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

Patronics
Copy link
Contributor

The "reset bands" item now allows the user to select the region, thus specifying which set of defaults to restore. This makes it easier for users in region 2 or region 3 to select the appropriate bands list without manually modifying entries.

Pull Request type

Please check the type of change your PR introduces:

  • [] Bugfix
  • Feature
  • [] Code style update (formatting, renaming)
  • [] Refactoring (no functional changes, no API changes)
  • [] Build-related changes
  • [] Documentation content changes
  • [] Other (please describe):

What is the current behavior?

The bands list currently defaults to only providing region 1 bands, which is confusing for users outside that region, where different bands are available, and the same bands have different minimum and maximum frequencies.

Issue Number: No active issues, but the confusion leads to rejected pull requests like #726

What is the new behavior?

  • Now, when choosing to reset default bands, the "are you sure?" dialog allows selecting which region's band defaults to use.
  • The button label is changed to "Reset bands/Select region" to improve discoverability of the new functionality.

Does this introduce a breaking change?

  • [] Yes
  • No

Other information

Screenshots of changed menu items:
Screenshot 2024-11-27 at 11 43 14 PM
Screenshot 2024-11-27 at 11 43 40 PM

@Patronics Patronics requested a review from zarath as a code owner November 28, 2024 07:44
Copy link
Collaborator

@zarath zarath left a comment

Choose a reason for hiding this comment

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

It's ok. I would perhaps made it more flexible with a list of defaults, but so it is better then just having that one default.

@zarath zarath merged commit c273079 into NanoVNA-Saver:main Nov 28, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants