Skip to content

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 requested a review from gadenbuie May 9, 2023 20:19
@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!

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()

3 participants