Change parserPopUp?

[Alex-Jordan]

Authors (including me) have written lots of exercises with something like:

$popup = PopUp(['?', 'red', 'orange', 'yellow'], 1);

where the '?' is a hack to make the select element's initial value not actually be one of the intended choices.

I'd like to change it so the author would write:

$popup = PopUp(['red', 'orange', 'yellow'], 1);

and using a technique like this there would just always be a placeholder initial value for the select. That value could be '?' but configurable as an option to pass to PopUp for an author who prefers something like "Choose one:".

The problem would be all those exercises that are already coded with a '?'. Or worse, those with something like a 'Choose one:'.

• Should I not even try this?
• Should I try this, and the consequences to all those existing exercises will be what they will be?
• Should I try this and code it to recognize things like '?' and 'Choose one:' as 0th entries, and automatically ignore them?
• Should I try this and simultaneously try to edit all the affected OPL problems?

[dlglin]

I think that this would be an improvement, but as you say it has implications for existing problems.

I wouldn't object if it causes some existing problems to effectively have two placeholder options, because it wouldn't make them unusable, and it's easy (though possibly tedious) to fix them.

A quick grep of the OPL shows

• 7146 instances of PopUp(
• 338 instances of PopUp("Select
• 279 instances of PopUp("Choose

These could theoretically be updated in an automated way, but there are many others that create an array and then pass that to PopUp, which would require more attention.

A couple of thoughts/suggestions:

• Should there be a flag that allows an author to disable the placeholder? Would anyone use that?
• The placeholder text should be configurable

[Alex-Jordan]

I was thinking to make the placeholder '?' be configurable as an option you could pass to PopUp. Did you mean something configured more globally?

Out of laziness I like '?' as the default since it doesn't need localization.

[somiaj]

Although I like the idea, I think it would be too disruptive to change the syntax of the array that is sent to PopUp. What about everyone's custom problems that will be broken with this change (not just the OPL)? This change might make me have to maintain my own parserPopUp.pl macro since that will be easier for me to deal with than asking all my instructors to change their code.

What about keeping the syntax the same (so users still have to manually input the placeholder as the first item in the list), but then use the method to disable the first entry in the list to make it not selectable after it has been changed? I am thinking about something like PopUp(['?', 'red', 'orange', 'yellow'], 1, disable_first => 1/0), where disable_first defaults to on (1). This way the syntax doesn't change, we don't need a new option to configure the placeholder (it will always be the first element of the array), but it could be configured if that is a place holder vs something that could be selected.

[Alex-Jordan]

I'm not suggesting changing the syntax. It would still be PopUp($arrayReference, $answer, options). And the correct answer would still be among the items in the $arrayReference. So I don't think current problems would be "broken".

What you suggest would actually break the existing problems that don't manually write a placeholder. If the correct answer is the 0th option, those problems would break.

[somiaj]

You suggested changing the array ref from ['?', 'red', 'orange', 'yellow'] to ['red', 'orange', 'yellow'], which to me breaks the syntax, as I don't think we can predict what everyone put in as the first entry for a place holder. I might have not fully understood you, but I read it as if the first item doesn't match a set list of possibilities, a new placeholder item would be appended to the start of the menu (which is what I see as breaking the syntax).

I don't think we should try to guess what the possible place holders people have chosen to use are, instead we should just assume that the 0th entry is the place holder (and this by default makes it configurable). I think this will break less cases, as I think it is probably more common people are using the first entry in the array as a place holder, so we should just treat it as such, and in the case they aren't, those few problems can disable that.

I did not think about the case that someone would choose the 0th option as correct. We could deal with this case, if the correct option is 0, then don't disable the first item since it needs to be selectable.

[Alex-Jordan]

Even if the 0th options is not the correct option, it still break things to have ['red', 'orange', 'yellow'] with 'red' treated by default as a placeholder. The value would start out as red, but if a student opens the dropdown, they will see 'red' is grayed out. Or if they temporarily select some other option, they cannot get back to 'red'. So even when 'red' is not the correct answer, there is a hint there to the user that it is not the correct answer.

I'm suggesting that if you have an exercise with ['Choose one:', 'red', 'orange', 'yellow'], then this change would make the select options be:

• ?
• Choose one:
• red
• orange
• yellow

And it's not great but also not terrible to have it like that. Or it could be coded to recognize a few select phrases like Choose one: and weed them out.

We are wrestling with how the actual options and the placeholder are different things, and imho we have mangled them together and have a legacy of that to deal with.

I think something should change, because right now there is no way (aside from human eyes on the code) to definitively discern if the 0th entry is an actual option or a placeholder. So when something like PreTeXt wants to know the actual options for the MC exercise, it has to assume the 0th entry is an option, which is leading to badness.

[dpvc]

Since PopUp() is a function that just creates a parser::PopUp instance, why not leave PopUp() as is and create a new function PopUpMenu(), or something similar, that has the new functionality? It could unshift the default value into the provided array (getting it from the options, if provide) and then call parser::PopUp->new() with the new array and options. That way all the old problems still work, but you get the new functionality with only the addition of a new function.

[Alex-Jordan]

I might be saying "change PopUp()" but actually mean changing something that it relies on, if that is what you mean.

Is it clear the new behavior would also be about disabling option 0? Like here?

[dpvc]

I might be saying "change PopUp()" but actually mean changing something that it relies on, if that is what you mean.

Only partly, as there will be a change to the underlying parser::PopUp class, but what I'm saying is that you have two front ends to the one back end. That is, you leave the PopUp() function as is, and add a new function PopUpMenu() that works the way you suggest. That way, all the existing problems work as they currently do, but your new problems (and any that you retro-fit) will use the new function.

You can implement PopUpMenu() as something like

sub PopUpMenu {
  my $choices = shift;
  my $correct = shift;
  my %options = {
    firstChoice => '?',
    @_,
    disableFirst => 1
  };
  unshift($options{firstChoice}, @$choices);
  return parser::PopUp->new($choices, $correct, %$options);
}

then modify the parser::PopUp::new() constructor to save the disableFirst option , if present, and the parser::PopUp::MENU() method to disable the first option when disableFirst is true. You could also add a cmp_preprocess() or cmp_postprocess() method that checks if the firstChoice value is still selected and have it produce a warning message indicating that a selection should be made.

That's it, though I haven't tested this. No need to try to figure out what options may be meant as the disabled choice, and no need to edit any existing problems. Just start using PopUpMenu(). Both that and PopUp() would be available from parserPopUp.pl.

[Alex-Jordan]

I can live with Davide's suggestion to make a new function. The new one should probably replace the old one in most places where the old one is used. But that will require a lot of manual editing of problem files and take time.

Before I go through with it, does anyone want to make any more suggestions for the name? PopUpMenu doesn't sit well for me. I was thinking DropDown.