Unstable - Bulk edit learning activities - Seeing 'This field is required' validation message

pcenov commented 4 weeks ago

Observed behavior

When I am bulk editing the learning activities for resources I see the resources selected with indeterminate status checkbox and the text 'This field is required'. If I uncheck the selected checkboxes and just check them again - I can save the change without any problems.

Expected behavior

To be further discussed.

Steps to reproduce the issue

Go to https://unstable.studio.learningequality.org/en/accounts/#/ and sign in
Open a channel with folders and all of the supported resources
Select several resources with the default activity type values as applied when they were uploaded
Attempt to bulk edit the learning activities

Additional information

https://github.com/user-attachments/assets/4b97f6b3-b723-43ca-9c26-296a9c6cb47b

Console logs

no errors observed in the console

Usage Details

OS: Windows 10, Ubuntu
Browser: Chrome, Firefox
URL: https://unstable.studio.learningequality.org/
Caused by: https://github.com/learningequality/studio/pull/4635

pcenov commented 4 weeks ago

@radinamatic

akolson commented 3 weeks ago

Tagging @marcellamaki for thoughts and clarity.

AlexVelezLl commented 3 weeks ago

This error is because all the options are in a indeterminate state because they are partially selected among all selected resources, so the modal treats this as if no option is selected. I wonder if we could treat indeterminate state as a valid state in the form validation.

marcellamaki commented 3 weeks ago

@AlexVelezLl that's a good suggestion. I guess my only other question is how to communicate to the user (and how we decide) if the indeterminate state and then clicking save updates so apply those changes to all, or if the user has to intentionally make a selection and check them.

In this state for example,

Do those tags with an indeterminate state get applied to all, or is it only the one that has the intentional check (i.e. Create) that is saved to all?

pcenov commented 3 weeks ago

@marcellamaki @AlexVelezLl we were also just discussing with Radina that when we are bulk editing folders or resources, then that indeterminate state can be confusing as one is not certain whether the tags with indeterminate state will get applied to all.

Here's an example with a bit puzzling 'Mixed' category displayed:

indeterminate state

I have to point out that when we have resources with mixed languages (or audience), we are offering a completely different approach, by simply informing the user that "The selected resources have different languages set. Choosing an option below will apply the language to all the selected resources" and all radio buttons are unchecked.

audience

In general it seems that when bulk editing items and there's a difference in terms of the previously set categories, activities, etc. it's best to explicitly state that to the user. We should also be consistent throughout all of the bulk editing modals.

AlexVelezLl commented 3 weeks ago

Hmm probably the indeterminate state of the checkboxes is not the best way to communicate this.

The current implementation allows that If we have two resources and three options A, B, C:

Resource 1 has option A set
Resource 2 has option C set

The modal will appear with option A and option C as undetermined. But if the user keeps options A and C as undetermined, and checks option B, then when saving:

Resource 1 will have option A and B set Resource 2 will have option C and B set

So selecting a checkbox while there are undetermined options would mean something like "Apply the current selection in addition to the options already set on the node." And if an indeterminate option is unchecked, it means that that option will be removed from the nodes that had that option selected.

But I think it's a difficult behavior to explain. Perhaps we can avoid the indeterminate state of the checkbox, show intederminate options as unchhecked, display the warning that there is a mixed selection and that the current selection (with just checked and unchecked options) is applied to all selected nodes, regardless of their previous state. And so as not to lose information about which options are partially selected, perhaps we can put something (a tooltip?) next to the options that are partially selected that indicates something like "X resources have this option selected.". But if there is an option that is selected in all selected resources we can show that option as selected, as we do in other modals.

@marcellamaki I think in any case this will need new strings :').

marcellamaki commented 2 weeks ago

@radinamatic @AlexVelezLl @AllanOXDi (cc @rtibbles)

I think we can mostly follow Alex's approach here. My suggestions would be that we:

remove "mixed" as a category tag; nothing should display here when the modal opens
no "indeterminate state" checkboxes shown at all. So nothings to display from the "existing" categories, if any.
If resources have different, existing categories (what would be currently shown as "mixed"), instead add helper text that is similar to the updated, post UX Writing helper text for the language example:

You selected resources that have different categories. The categories you choose below will be added to all selected resources. This will not remove existing categories.
no changes to the underlying behavior
when a checkbox is selected, the chip appears matching that category

I wonder if the validation should appear at all. Maybe the better behavior is to not show a validation, but to have the "save" button only be active if one or more boxes is checked? else, cancel only.

Thoughts anybody?

@AllanOXDi - we will try to finalize promptly so you can get started on this!

rtibbles commented 2 weeks ago

That seems right - it means we push bulk editing more towards a kind of 'apply changes' workflow, rather than actively, reactively, trying to edit lots of resources at once.

marcellamaki commented 2 weeks ago

yes, it seems more aligned with the other modal for move/update that we (you) are currently working on, Richard.

If we get feedback from users that people do want it to be more dynamic in terms of adding AND removing, I'd be amenable to changing it a future release. But for now I think this is okay, and less complicated/confusing at least for the first version of this going out into the world.

radinamatic commented 2 weeks ago

Sounds like a good direction to take at this time 👍🏽 Agreed on waiting to hear from users whether is there any unmet need in terms of both adding and removing.

marcellamaki commented 2 weeks ago

great - so, for next steps.

@AllanOXDi - I will create a PR to add the helper text string based on the conversation here. It looks like the validation is pulling from an existing string re: required text, so that doesn't have string implications. This way we can go forward with translation, and there isn't as much of a time crunch to get a PR really soon. For the rest of the issue the steps would be:

Don't display a "mixed" tag as a category
Don't display any indeterminate state checkboxes
If resources have different, existing categories (what would be currently shown as "mixed"), add a condition to display the new helper text:

You selected resources that have different categories. The categories you choose below will be added to all selected resources. This will not remove existing categories.

the "save" button only be active if one or more boxes is checked, but no validation is required (and no error message)

If you have any other questions when you get going on this, I'm happy to talk it through more!

marcellamaki commented 2 weeks ago

Removing the user strings tag for string freeze/translation need tracking purposes, as the string was added in #4689

learningequality / studio