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

Give better hint for checkbox questions created with question() in try_again message #783

Merged
merged 6 commits into from
May 9, 2023

Conversation

rossellhayes
Copy link
Contributor

@rossellhayes rossellhayes commented May 9, 2023

This PR extends #732 to apply to checkbox questions created by question() in addition to checkbox questions created by question_checkbox(). It does this by changing the default value of try_again in question() to NULL. If try_again is NULL, question() determines the default message based on the question type.

Closes #782.

@rossellhayes rossellhayes added the type: enhancement Adds a new, backwards-compatible feature label May 9, 2023
@rossellhayes rossellhayes self-assigned this May 9, 2023
@gadenbuie
Copy link
Member

Can you explain a bit more about the decision to add a new argument?

@rossellhayes
Copy link
Contributor Author

@gadenbuie Because question() can create both radio questions and checkbox questions, it needs to have two different default "try again" messages: one for radio questions and one for checkbox questions.

An alternative solution would be to change the default value of try_again to NULL and determine the default within the function. Would you prefer that?

@gadenbuie
Copy link
Member

An alternative solution would be to change the default value of try_again to NULL and determine the default within the function. Would you prefer that?

Yeah, that would be better. When you call question(), you're creating a unique question – only one try_again will apply.

…value of `NULL`

Move logic to determine default message inside the function
@rossellhayes
Copy link
Contributor Author

That makes sense, refactored in 316d85f

Copy link
Member

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look great, thanks!

NEWS.md Outdated Show resolved Hide resolved
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
@rossellhayes rossellhayes merged commit 9e18b55 into main May 9, 2023
@rossellhayes rossellhayes deleted the feat/try_again_checkbox-message branch May 9, 2023 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Adds a new, backwards-compatible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved try_again checkbox question message does not apply when question is created by question()
2 participants