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

Multiple choice #1019

Merged
merged 4 commits into from
Mar 20, 2024
Merged

Multiple choice #1019

merged 4 commits into from
Mar 20, 2024

Conversation

Alex-Jordan
Copy link
Contributor

This was primarily meant to address #1017, so that now when there is a PopUp and a nonegative integer is declared as the answer, you have the same behavior as with RadioButtons where it is assumed to be an index, even when it matches one of the actual answers. But you can use noindex => 1 to override that.

Several other changes happen too. An HTML option (inside an HTML select) can have value and label attributes. If there is text content inside the option, that is used as the default for the value and label attributes. If both attributes are specified, I think there is no significance to the text children of the option. Anyway, in the manner of @drgrice1's revision to parserRadioButtons in the last release (or the one before?) this now carefully separates the option text, option value, and option label. The details are slightly different owing to how label means something different here. But much of this was copying chunks from parserRadioButtons.pl and then here and there adjusting.

I'm updating the PTX output to be more semantically correct. It will require new things on the PTX side, but that should be OK.

Lastly, I thought it would be convenient to make a parent parserMultipleChoice.pl file to load the four multiple choice macro files we have now.

@pstaabp
Copy link
Member

pstaabp commented Feb 20, 2024

I've been thinking it would be nice to have a replacement for PGchoicemacros.pl for matching problems and has the API's similar to CheckboxList, RadioButtons. Anyway, I would think it could be called parserMultipleChoice.pl, but if we use this filename to pull in all similar macros, it could be parserMatching.pl.

@Alex-Jordan
Copy link
Contributor Author

There is some discussion of this at #843.

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

I think this looks good. I am not sure how useful the values option is for this in general. Perhaps you have a particular usage for PTX in mind with that. The main objective with radio buttons with the values option was to give something more meaningful that can be displayed in the past answers. For popups that isn't really an issue.

One thing that I would like to do with the parserPopUp.pl macro is to stop using a native browser select, and use a javascript implementation. The advantage of doing that would be that you could actually have math equations in the drop down.

macros/parsers/parserPopUp.pl Outdated Show resolved Hide resolved
macros/parsers/parserPopUp.pl Outdated Show resolved Hide resolved
Copy link
Member

@drgrice1 drgrice1 left a comment

Choose a reason for hiding this comment

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

One other clean up item.

macros/parsers/parserPopUp.pl Outdated Show resolved Hide resolved
macros/parsers/parserPopUp.pl Outdated Show resolved Hide resolved
@pstaabp
Copy link
Member

pstaabp commented Mar 20, 2024

I think this looks ready to go now. @drgrice1, ready to merge?

@drgrice1
Copy link
Member

Yeah, this can be merged.

@taniwallach
Copy link
Member

I added a comment to the release notes about this being a breaking change for some problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants