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

Added an auto-pause preference, added Esc to unpause, & dup'd preferences to solver more menu #466

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

adamthedog
Copy link
Member

Closes #389

@mdirolf
Copy link
Member

mdirolf commented Feb 1, 2024

Overall this looks great to me! I think my only notes are:

  • Build is failing due to eslint checks. I don't mind taking a look at this if you aren't able to
  • I know you asked about it but I think in the end it probably does make sense to add a new SolverPreferencesList(or something) component that takes PrefSettingProps and returns a fragment filled with all of the <li>s - will probably just help to avoid future mistakes. Again I don't mind doing that if you don't have time!
  • I think I'd change the text from "Pause solving when clicking away from the page" to "Automatically pause solving when page loses focus" - just makes it a tiny bit more clear in cases where somebody has an OS w/ focus follows mouse, for example.

Thanks for the contribution - I really appreciate it and can't wait to see it live :)

@mdirolf mdirolf merged commit 6d79523 into crosshare-org:master Feb 2, 2024
1 check failed
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.

Make auto-pause an optional feature for constructor and/or solver?
2 participants