[#11085] Instructor edit questions: tweak option order for generating answers from teams

[tlhuynh]

Fixes #11085

Outline of Solution

The goal is to reorder the list from "Or, generate options for the list of all" checkbox for the multiple-choice type questions.

The fix was to reorder the strings inside PARTICIPANT_TYPES to match with the expected order.

The files edited were:

src/web/app/components/question-types/question-edit-details-form/mcq-question-edit-details-form.component.ts

src/web/app/components/question-types/question-edit-details-form/msq-question-edit-details-form.component.ts

[daongochieu2810]

Changes look good to me! However please remove changes made in .vscode/settings.json, angular.json and the alarmingly large package-lock,.json

[moziliar]

Do remove file .vscode/settings.json (I suppose you should add it in your .gitignore, preferably globally too), and git restore both angular.json and package-lock.json as mentioned above, which were a result of your npm install.

[tlhuynh]

Alright! I made the changes. I'll keep this in mind for the next PR. Regarding the package-lock.json and angular.json, what do you guys normally do to avoid big changes added to those files due to npm install?

[moziliar]

@tlhuynh Before you commit, you should check your git status (and even git diff) to ensure that you are going to commit the intended changes; if you find changes to files like package-lock.json, you could just git restore it, since the file is about locking the dependency versions in your package.json and you shouldn't really expect it to be different from the master unless you add in new dependency (and lock again). For changes to angular.json, you could just simply omit it when committing.

Of course, the cause of the additional line in angular.json is that you enabled angular's analytics by default (becoz Google); you can do ng analytics off to manually turn it off, or just indicate N when prompted upon npm run start (which in turn runs ng serve that normally prompts you so anyways).

[moziliar]

Not sure if we should explicitly add this here.. Ideally, every dev should kind of add it anyways.
@rrtheonlyone should we add vsc gitignore in this PR? I see that IDEA is added but not vsc.

[moziliar]

I think for now, you could try to add this line to your own global .gitignore first instead of project local.

[tlhuynh]

Got it. I will get to it now.

[moziliar]

Maybe you should reorder the options further:

student -> student exclude self
team -> team exclude self
own team exclude self -> own team (reversed?)

[tlhuynh]

@moziliar I see I see. Thank you so much. I'll make sure to pay attention to this more from now on.

[moziliar]

One thing to note is that for UI changes, it is best to attach screenshot of all the affected components.

[tlhuynh]

Do you mean for the PR description? Like screenshots for how it was before and how it looks after the changes? I thought about it but wasn't sure if it was necessary. Will do, if I work with UIs again.

[moziliar]

LGTM. @damithc do you want to vet the ordering of the options too?

[damithc]

@damithc do you want to vet the ordering of the options too?

Yes please. Let me know when a screenshot has been posted.

[moziliar]

@tlhuynh typically, a screenshot is needed for UI related changes. Do attach screenshots for both dropdown (I see two are changed and only these two are needed)

[tlhuynh]

@damithc , @moziliar

Here are the screenshots for the changes:

This one is for the MC-single answer

And, this one is for the MC-multi answers

[damithc]

his one is for the MC-single answer

Not what I was expecting when I posted the original issue. Isn't it easier for the user if we pair x and x (excluding ...)?

e.g.,

students
students (excluding self)
...

[tlhuynh]

@damithc
That was the original fix, but @moziliar wants to see if we can reorder it further and put them into groups (with self and excluding self). I can always reorder them depend on how you guys want them.

[moziliar]

@tlhuynh I didn't imply grouping by the way; instead, I was just pointing out that the self and excluding self ordering of the last pair was reversed.

[tlhuynh]

@moziliar
Ahh, I see I see. I misunderstood, then. I thought you wanted to separate them. That was my fault. Making the change now.

[tlhuynh]

@damithc
Here are the new screenshots:
For MC-single answer:

For MC-multi answers:

[damithc]

For MC-single answer:

Looks good to me 👍

[daongochieu2810]

lgtm