feat: Added multiline attribute to string input element

[eliotwrobson]

See title, resolves #3064. Also updates the example question to make it cleaner and more closely match other examples.

[jonatanschroeder]

The submitted answer looks weird, probably worth replacing code with pre in this case:

Correct answer also looks weird, might want to use an inline-block or something similar...

[eliotwrobson]

The submitted answer looks weird, probably worth replacing code with pre in this case:

I tried changing this to a pre, but it looked exactly the same.

Correct answer also looks weird, might want to use an inline-block or something similar...

Instead of an inline-block, I actually applied the same unicode-escaping to the correct answer as to the student answer. Although a change in previous behavior, this makes the element behave more consistently and it's easier to see if you're making a mistake because none of the characters are invisible anymore.

I'll put a note in the docs about the correct answer / submitted answer displaying weird if they are too long. I think in most cases, people who are using the multiline will hide these panels.

[jonatanschroeder]

Instead of an inline-block, I actually applied the same unicode-escaping to the correct answer as to the student answer. Although a change in previous behavior, this makes the element behave more consistently and it's easier to see if you're making a mistake because none of the characters are invisible anymore.

Keeping the characters not invisible is an interesting approach, but could be a bit confusing for users (particularly students). In essence, if the student sees a character they don't recognize they may believe that the representation (and not the character itself) is part of the answer. I still think that an inline block makes sense here. You can have horizontal scroll to avoid confusing word wrap with line breaks in case of multiline.

I'll put a note in the docs about the correct answer / submitted answer displaying weird if they are too long. I think in most cases, people who are using the multiline will hide these panels.

I don't think recommending hiding should be done so quickly. If the display is weird, it makes more sense to change the display than to expect the instructor to hide it.

[eliotwrobson]

Keeping the characters not invisible is an interesting approach, but could be a bit confusing for users (particularly students). In essence, if the student sees a character they don't recognize they may believe that the representation (and not the character itself) is part of the answer. I still think that an inline block makes sense here. You can have horizontal scroll to avoid confusing word wrap with line breaks in case of multiline.

I think we can discuss this in the upcoming meeting, but the current behavior is to display escaped characters to avoid showing students an answer with invisible characters. Having scrolling for multiline output makes sense, but I'm not sure which display would be preferred by others.

[eliotwrobson]

Note to self: I think the last remaining question for this PR is whether unicode escaping should be enabled as an option for what is shown to the student. I disabled it here, but that has a small chance of confusing people since that was the prior behavior, though I don't think this is a big deal. Was anyone relying on this feature?

[mwest1066]

Could we default this to true even for multiline input or is there a problem with CR/LF characters?

It sounds like textarea elements will always send us CR+LF line endings, so I think it should work to just not escape the CR+LF pair on the end of each line, but to escape everything else within the line (including any singleton CR or LF characters that aren't part of a CR+LF pair).

[jonatanschroeder]

Personally I'm in favour of normalizing both correct answer (in prepare) and submitted answer (in parse) to LF, mostly because I believe many instructors will likely use \n in their setting of the correct answer, and trying to explain this difference may be something we want to avoid. In essence, replace /\r?\n/ (or, if we're feeling generous, /\\s*\n/) with \n in parse. In either case, I agree that the line break should not be escaped, whether or not we keep the CR.

[eliotwrobson]

Personally I'm in favour of normalizing both correct answer (in prepare) and submitted answer (in parse) to LF, mostly because I believe many instructors will likely use \n in their setting of the correct answer, and trying to explain this difference may be something we want to avoid. In essence, replace /\r?\n/ (or, if we're feeling generous, /\\s*\n/) with \n in parse.

I'm mostly in agreement with this, and the current behavior in this PR is to normalize the submitted answer to LF (https://github.com/PrairieLearn/PrairieLearn/pull/9438/files#diff-e3c5943c659a94abbfdbf1f52defa85320b1271a71523abfb1a4b54f17131a9aR254). It's straightforward enough to add this behavior to the correct answer used in grade as well (doing this in grade rather than parse is consistent with how other input transformations are handled).

Could we default this to true even for multiline input or is there a problem with CR/LF characters?

I'm actually in favor of keeping the current behavior of an all-or-nothing escaping of the Cc (unicode control) characters (so by default, no escaping for multiline input). The reason for this is that the point of the multiline input is to faithfully display the input string with control characters (so newlines skip to the next line, tabs indent, etc.). In fact, I think newlines are the most common examples of the control characters, and it's much less confusing than with the one-line string input if these are present (looking back, I think one of the main reasons for the previous escaping behavior was probably just to avoid displaying the student output on multiple lines).

To be clear, outside of the Cc and Cf character codes, other weird unicode stuff is handled by normalize-to-ascii option, so changing the escaping of control characters won't introduce weird issues outside of changing the display.

[mwest1066]

I'm mostly in agreement with this, and the current behavior in this PR is to normalize the submitted answer to LF (https://github.com/PrairieLearn/PrairieLearn/pull/9438/files#diff-e3c5943c659a94abbfdbf1f52defa85320b1271a71523abfb1a4b54f17131a9aR254). It's straightforward enough to add this behavior to the correct answer used in grade as well (doing this in grade rather than parse is consistent with how other input transformations are handled).

Sounds great!

Should we also normalize singleton \r characters (not followed by an \n) to \n?

I'm actually in favor of keeping the current behavior of an all-or-nothing escaping of the Cc (unicode control) characters (so by default, no escaping for multiline input). The reason for this is that the point of the multiline input is to faithfully display the input string with control characters (so newlines skip to the next line, tabs indent, etc.). In fact, I think newlines are the most common examples of the control characters, and it's much less confusing than with the one-line string input if these are present (looking back, I think one of the main reasons for the previous escaping behavior was probably just to avoid displaying the student output on multiple lines).

To be clear, outside of the Cc and Cf character codes, other weird unicode stuff is handled by normalize-to-ascii option, so changing the escaping of control characters won't introduce weird issues outside of changing the display.

I think there are two main problems we face:

1. How do we help instructors write questions that don't suffer from unexpected grading behavior caused by students inputting unexpected characters?
2. How do we help instructors (and students) see exactly what was input, especially in cases where the submission was graded as incorrect but it looks visually correct?

For problem (1) I think that normalizing CR+LF and CR to LF is probably sufficient, as you say. For (2), the normalize-to-ascii option handles this, but it defaults to false and so is of no use post hoc and there are also legitimate reasons for not using it. To solve (2), some ideas are:

1. Escape unicode Cc and Cf sets in multiline mode, except for TAB and LF (assuming we normalize CR to LF). Of course this would only be a partial solution.
2. Add some other way for instructors (and students) to see exactly what was input. For example, this could be a "?" button next to the input in the submitted answer that shows a popover with input displayed with every non-printable-ascii (that is, everything outside of 0x20 to 0x7E) rendered as <U+....>.

What do you think? We could defer this to a later PR, but I do think it becomes more important with multiline input because students are more likely to be copy/pasting blocks of text that may well contain characters that the question author did not expect.

[eliotwrobson]

I think the popover might be the way to go, similar to the way that the symbolic input element shows the raw input to the student (this would make it even easier to implement since we can just copy the HTML used in the template there). This sets more consistent expectations about behavior across elements.

As tempting as it is to do proposed solution (1), I think this runs the risk of adding more layers of complexity to this without completely solving the problem, which was my initial reservation with any limited replacement type solution. I think this is a good discussion item for the meeting.

I actually think it might be fine to defer this to a later PR. The blocks of text people might be copying will contain more control characters (like newlines), but I don't think this change will invite copying larger blocks of more exotic unicode characters. We can discuss during the meeting tomorrow, but if it's a low-overhead thing and doesn't expand the scope of this PR too much I might be able to add it before another round of review.

[mwest1066]

Looking at pl-symbolic-input it looks like it displays raw_submitted_answer without any escaping? I think other elements like pl-number-input also do the same thing (show the raw submitted answer without escaping)? Maybe we should be showing a popover with raw_submitted_answer displayed with Cc and Cf escaped by default, and add a collapsable "details" element in the popover that shows the input with everything escaped except 0x20-0x7e? As you say, having a standard display for this across elements would be great.

[eliotwrobson]

Yes, the pl-symbolic-input doesn't have any special behavior for unicode escaping in its display of the raw answer. I agree standardizing the display in this case would be valuable. This will require a little more thought (and namely, a second PR), but I think we could do this with pretty low overhead overall by sharing the logic. As long as what's going on here currently is semi-reasonable (I'd argue it's probably an improvement over the previous behavior), I should be able to handle this in a short follow-up.

[eliotwrobson]

Made an issue for this follow-up: #9717

Given the scope, it might not be a super fast follow-up (I would be hesitant to deploy unifying logic like this before summer), but discussed during the meeting and major changes to the escaping behavior are a little orthogonal to the point of this PR.

I think the bar for review on the present PR should be just be adding the multiline behavior and making it no-more-confusing than the present escaping, while compatible with the later planned changes. For this, I will remove the element attribute for escaping on the display (since ultimately, we want to display with a popover that shows all the information instead of adding an attribute), and add a popover with a barebones escaping of the string inputted by the student (using the same visual language as the raw display on symbolic input). We can defer larger changes / discussion to the linked issue.

[eliotwrobson]

Initial changes in this direction here, still have to work out the kinks: 1e734b6

[mwest1066]

I like your proposed "no-more-confusing-than-present" strategy.

[eliotwrobson]

Overall, I think this PR is almost ready to go! The only things left are messing with formatting on a few things, which I would greatly appreciate a small amount of guidance with.

• Right now, I'm not sure whether the display of the suffix looks weird or not, the spacing is slightly different than the top label, but I'm not sure if this is standard.
• Is there a way to avoid having to put spaces in the first code tag in this popover? When I remove the spaces, the < and > don't display for some reason:

Other than that I think the behavior here is pretty reasonable and overall makes this element more intuitive.

[jonatanschroeder]

Is there a way to avoid having to put spaces in the first code tag in this popover? When I remove the spaces, the < and > don't display for some reason:

Are you escaping the lt/gt? Assuming this is inside an argument, you may need to escape twice (< for <).

[eliotwrobson]

The code for this popover is in the lines just above. I didn't need to escape twice to get it to work, but it stops working once I get rid of the surrounding whitespace.