Skip to content

Conversation

somiaj
Copy link
Contributor

@somiaj somiaj commented Jun 19, 2025

  • Make links with pg_files start with a leading slash.
  • Honor the showCorrectAnswers setting and don't force it to be 2.
  • Make pg_files thirdPartyAssessts not force CDN use.
  • Remove the no longer needed s/#/@/g from version strings.
  • Some minor code cleanups.

somiaj added 4 commits June 19, 2025 15:32
* Make links with `pg_files` start with a leading slash.
* Honor the showCorrectAnswers setting and don't force it to be 2.
* Make pg_files thirdPartyAssessts not force CDN use.
* Remove the no longer needed `s/#/@/g` from version strings.
* Some minor code cleanups.
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.

This is not the correct baseURL approach. The key for the correct way to do this is to use the url_for method and route names (with appropriate stash values in some cases). I will put in a pull request at some point that fixes this. I have it implemented, but I am waiting for some of my other pull requests to be merged before I put it out there.

showCorrectAnswers => $inputs_ref->{showCorrectAnswers} ? 2 : 0,
num_of_correct_ans => $inputs_ref->{numCorrect} || 0,
num_of_incorrect_ans => $inputs_ref->{numIncorrect} || 0,
showCorrectAnswers => $inputs_ref->{showCorrectAnswers} && $displayResults || 0,
Copy link
Member

Choose a reason for hiding this comment

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

This is still not right. The value of showCorrectAnswers should be numeric and should be one of 0, 1, or 2. This does not allow for a value of 2 which is the case that correct answers are shown without the button to reveal the correct answers.

@somiaj
Copy link
Contributor Author

somiaj commented Aug 5, 2025

I will split this up, and remove the baseURL update, though I would like your baseURL fix when you are ready to make a PR with it.

@somiaj
Copy link
Contributor Author

somiaj commented Aug 5, 2025

Closing, in favor of a better baseURL approach.

@somiaj somiaj closed this Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants