Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
pcenov opened this issue Aug 16, 2024 · 12 comments · Fixed by #4714 or #4737
Closed
Assignees
Labels
bug DEV: frontend P0 - critical Priority: Release blocker or regression

Comments

@pcenov
Copy link
Member

pcenov commented Aug 16, 2024

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

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

Additional information

Bulk.edit.activities.mp4

Console logs

no errors observed in the console

Usage Details

@pcenov
Copy link
Member Author

pcenov commented Aug 16, 2024

@radinamatic

@radinamatic radinamatic added bug P0 - critical Priority: Release blocker or regression DEV: frontend labels Aug 16, 2024
@akolson
Copy link
Member

akolson commented Aug 19, 2024

Tagging @marcellamaki for thoughts and clarity.

@AlexVelezLl
Copy link
Member

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
Copy link
Member

@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,
Screenshot 2024-08-21 at 10 59 52 AM
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
Copy link
Member Author

pcenov commented Aug 21, 2024

@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
Copy link
Member

AlexVelezLl commented Aug 21, 2024

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
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

marcellamaki commented Aug 29, 2024

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:

  1. Don't display a "mixed" tag as a category
  2. Don't display any indeterminate state checkboxes
  3. 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.

  1. 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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug DEV: frontend P0 - critical Priority: Release blocker or regression
Projects
None yet
7 participants