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

[FR] Some settings UI improvements #882

Closed
shumadrid opened this issue Mar 20, 2025 · 1 comment · Fixed by #886
Closed

[FR] Some settings UI improvements #882

shumadrid opened this issue Mar 20, 2025 · 1 comment · Fixed by #886

Comments

@shumadrid
Copy link
Contributor

shumadrid commented Mar 20, 2025

I did see a recent #878 issue that already made improvements. I have some more suggestions:

  1. Show invalid textbox states for invalid settings instead of sending notices (which were deleted in fix(settings): remove ability to enter non-number in number input boxes #880 PR, but in the current implementation you can still specify negative numbers)
  2. Set all default settings as placeholders (even after save, by naively checking if the saved setting matches the default one, and then setting a placeholder instead of .setValue())
  3. Set a 80ms delay between saving the toggle settings that enable other settings, and refreshing the display, so that the toggle doesn't jump from one state to another but looks at least a bit like the others (imperfect fix but easy)

I can submit a PR for this if the maintainer agrees.

@Vinzent03
Copy link
Owner

To your specific points:

  1. I would not overengineer it, putting in negative values just makes no sense and I don't think people come up with the idea of doing it. So I would just accept that.
  2. I like the idea. This allows a more visual representation of resetting to the default value than the current one with the zeros:
    Image
  3. Yeah I noticed that issue myself already. This feels kinda hacky, but should work pretty fine.

If you want to work on a pr, that would be great!

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 a pull request may close this issue.

2 participants