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

Add local storage for time signature and reset option #27

Closed
dcallus opened this issue Jul 9, 2022 · 4 comments · Fixed by #31
Closed

Add local storage for time signature and reset option #27

dcallus opened this issue Jul 9, 2022 · 4 comments · Fixed by #31
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@dcallus
Copy link
Contributor

dcallus commented Jul 9, 2022

Please add local storage options for time signatures so that the user's preference is saved.
Follow practices in PR#25#25

Rename the RESET TO DEFAULTS button in the settings menu to 'RESET ALL SETTINGS' and make it apply to the play settings (as well as those already reset in the settings menu). i.e.
make sure it also:

  1. Resets tempo slider to 60 - 120
  2. Resets time signature to 4/4
@dcallus dcallus added enhancement New feature or request good first issue Good for newcomers labels Jul 9, 2022
@ricci2511
Copy link
Contributor

ricci2511 commented Jul 9, 2022

@dcallus Hi again, since I did the previous LocalStorage issue I think I could look into this too.
From my understanding you have a DEFAULTS object in App.tsx and a resetAllSettings function to reset to those values.
Should I add a tempo slider and time signature key value pair in that DEFAULTS object and handle its reset in the resetAllSettings function?

That would be one solution to the problem, but I'm not sure if you have something else in mind.

@dcallus
Copy link
Contributor Author

dcallus commented Jul 9, 2022

Hmmm. I'm not sure actually. I'll have a think tomorrow and let you know. Thanks for asking.

@kristersd
Copy link
Collaborator

@dcallus I have added an PR recently, storing in DEFAULTS would not make sense, as they are defaults for the drawer itself. I have implemented reset settings with an custom event, which I guess is the best solution, without refactoring a lot of the app, and moving to global state.

@dcallus
Copy link
Contributor Author

dcallus commented Jul 9, 2022

Thanks @chrisd1999

Looks great. I'll have a proper look tomorrow. I need to go to bed shortly as its around midnight here in Scotland, can't believe how busy it's been today with everyone chipping in :)

Thanks so much for your contribution.

edit: managed to to get my branch stuff working :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants