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

Disable judges' game panel buttons if required inputs aren't set. #17

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

timparenti
Copy link
Member

Specifically, "start game" and "set flags" buttons.

Resolves #15.

Specifically, "start game" and "set flags" buttons.

Resolves cmukgb#15.
@timparenti
Copy link
Member Author

Some amount of form validation can probably come out of postForm() with this change, as well. Feel free to ask me to tackle that, or to tackle it yourself, @michaelfelixmurphy.

@michaelfelixmurphy
Copy link
Member

This looks fine, but my only concern is that, if the button is disabled, there's no indication why. I thought about this because of course now we could remove the requirement checking code from postForm(), because we can never trigger it - but then we'll never show the required error message - so you have no way of knowing why the button is disabled. I can't think of an easy solution for this at the moment. Maybe it's not that big of a problem.

@timparenti
Copy link
Member Author

timparenti commented Mar 4, 2019

I'll keep thinking on it this week, then. Initial thought is a combination of:

  • A tooltip on the button whenever it's disabled, and
  • The existing error stylization in postForm() being moved into some sort of "if you try to click it anyway" function.

Let's keep this open while I iterate on that work, since that kinda goes hand-in-hand.

@michaelfelixmurphy
Copy link
Member

michaelfelixmurphy commented Mar 4, 2019 via email

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