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 minimal word length setting #17

Merged
merged 11 commits into from Oct 26, 2018

Conversation

rspilker
Copy link
Contributor

Adds a new setting to the setup screen to select the minimal word length.

Shorter words entered by the user will not be checked. The grid above the board displays a white 'x' instead of the number and the results page will not display shorter words.

Current possible values as '2+', '3+' and '4+'.

@bennorichters
Copy link
Owner

bennorichters commented Oct 24, 2018

I like the idea. But I have some issues with this:

  • It breaks the unit tests. They do not compile anymore. They would need to be changed to cover the new behavior anyway.
  • With longer minimal length of allowed words, chances increase that there are no solutions at all. Does this need to be addressed?
  • I think you found the best possible icon for the slider, but it is definitely less intuitive than the other two. But this is not a deal breaker for me.

@rspilker
Copy link
Contributor Author

I'll fix the tests. Forgot about those. Possibly, we need to regenerate the board of there are no or not enough solutions

@rspilker rspilker force-pushed the minimallength branch 2 times, most recently from d07983c to 5286101 Compare October 25, 2018 17:57
@rspilker
Copy link
Contributor Author

The tests are fixed now

@rspilker
Copy link
Contributor Author

The commits in between are already in master. The only relevant changeset is 4c9e056

@bennorichters bennorichters merged commit 4c9e056 into bennorichters:master Oct 26, 2018
@rspilker rspilker deleted the minimallength branch October 27, 2018 15:30
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.

None yet

2 participants