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 confirm and cancel commands for daily games #78

Merged
merged 2 commits into from
Feb 2, 2025

Conversation

leandwo
Copy link
Contributor

@leandwo leandwo commented Jan 28, 2025

This PR introduces two new commands: confirm and cancel, to handle daily game functionality referenced in #76.

Please let me know if you'd like me to change anything.

Changes

  • Added confirm and cancel command by clicking their respective buttons, similar to the draw and resign commands.
  • Updated two imports to use ES module syntax to resolve test execution issues in my environment.
cancel_confirm_commands.mov

@leandwo
Copy link
Contributor Author

leandwo commented Jan 28, 2025

One issue is that confirm and cancel are a bit long to type out. I was thinking of maybe adding a feature to cycle through the commands with tab instead of the arrow keys or if there is only 1 matching command then press enter to autocomplete the only matching command.

@everyonesdesign everyonesdesign enabled auto-merge (rebase) February 1, 2025 19:51
@everyonesdesign
Copy link
Owner

Thank you for your PR, @leandwo

I tested it and I couldn't merge it for now because type: "module" was incompatible with the e2e tests for now. I'm migrating the tests to TypeScript to make it work (#79) as soon as it's ready I'll let you know. You'll probably have to rebase the PR.

I was thinking of maybe adding a feature to cycle through the commands with tab instead of the arrow keys or if there is only 1 matching command then press enter to autocomplete the only matching command.

Yeah, this idea sounds good, maybe something to explore.

@everyonesdesign
Copy link
Owner

The master is updated, could you, please, rebase this PR, @leandwo ?

auto-merge was automatically disabled February 2, 2025 04:23

Head branch was pushed to by a user without write access

@leandwo
Copy link
Contributor Author

leandwo commented Feb 2, 2025

@everyonesdesign Thanks for figuring that out! I rebased onto your master branch.

@everyonesdesign everyonesdesign merged commit 25d4b2f into everyonesdesign:master Feb 2, 2025
1 check passed
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.

2 participants