-
Notifications
You must be signed in to change notification settings - Fork 2
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
Anagram #7
Anagram #7
Conversation
…into part-of-speech
get_part_of_speech_words now expands contractions before tokenizing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is SO GOOD. I don't even have any of my usual nitpicky code style feedback: the code is beautifully easy to read, documented, well structured, and tested. Fantastic work.
I removed the dependency_parsing
module taken from Gender Analysis Toolkit, as in the end this project wasn't using it.
I have some comments, but these should be taken as directions for further work and refinement -- nothing here prevents me merging.
- Let's add mode where the pool of available letters is separated by words (so it's a classic anagram per word, not an anagram of all the words all mixed together. As is, the game is pretty hard!
- After hitting "Give up" add a distinction between in "Words found" between words that the student found and those given by the answer key.
- "Extra words" isn't really clear: took me a little experimenting to figure out what that was about. Plus it shows the confetti animation, even though those aren't correct answers.
- Needs some responsive design work: I think this interface is totally feasible to make work at the narrower screen sizes, but right now the main columns in the interface squish together instead of stacking.
- Add instructions in the placeholder place and show that modal by default when you open the game.
- The hover definitions feature wasn't very clear to me when I first hovered: I was confused about what I was seeing. Maybe that could be more apparent in the interface or labeled better? (Even "other definitions" would make it clearer)
Merging. Fantastic work. (If folks on the team could make some follow up issues from the comments above, would be great...)
In this pull request, we have added an anagram game with the features of:
@ryaanahmed