learningequality / kolibri

Kolibri Learning Platform: the offline app for universal education
https://learningequality.org/kolibri/
MIT License
808 stars 682 forks source link

Improve user experience when selecting lesson recipients in Coach #9097

Open MisRob opened 2 years ago

MisRob commented 2 years ago

Observed behavior

The relationship between the "Entire class" radio button and the "Individual learners" checkbox when selecting lesson recipients in Coach is not very clear at a first glance. When I select "Individual learners", "Entire class" radio button stays selected until I select at least one learner (makes sense in a way but still feels pretty confusing):

lesson-recipients

Expected behavior

We decided to

as illustrated on the new design:

recipients

Notice that the "Individual learners" checkbox should be indeterminate when only some individual learners are selected.

User-facing consequences

It's confusing

Steps to reproduce

When signed in as a coach

  1. Create a lesson
  2. Navigate to "Coach" --> "Plan" --> "Lessons"
  3. Select the lesson
  4. On the lesson page, click on the "Options" dropdown and select "Edit details"

Context

andersan commented 2 years ago

Hey Michaela! This issue is really detailed, is it still necessary to fix?

Looking for a good first issue on the project

bjester commented 2 years ago

Hi @andersan,

Thank you for interest in contributing! This issue is still valid.

Before addressing it, please follow the Getting Started documentation for Kolibri development. Please note, there's currently an error in the documentation where it says Install Node.js (version 10 is required); version 16.x is required and the commands below the mention instruct you to install that correctly.

The developer documentation has other valuable information regarding Kolibri. In addition, we also have end-user documentation.

Please base your work off the develop branch to address this. This issue exists within Kolibri's coach plugin, and I believe this is the Vue.js component.

Regards, Blaine

andersan commented 2 years ago

@bjester thanks for the info, appreciate it. today I got it up and running and the project seems really useful, so I'll be happy to contribute to it.

https://github.com/learningequality/kolibri/pull/9173 BTW I added a pull request with two minor edits to that getting started doc, as a first contribution and to learn the process for making PRs in this project.

Is there a designated person or people to assign as reviewers? I can see you're a reviewer on some items, and rtibbles is reviewer on almost all of them.

bjester commented 2 years ago

@andersan Thanks for updating the documentation! I approved and merged your PR.

As far as designating a reviewer, the Learning Equality dev team tries to stay on top of these, but you can mention me in a comment to tag a reviewer

andersan commented 2 years ago

Thank you for being so quick on this! Will go after this issue sometime in the next couple of weeks. Just want to play around with the tool and explore installing it on a Raspberry Pi at a school I volunteer at first to get a feel for how it all works.

andersan commented 2 years ago

Just getting to this now 😅

Is there an example of where the indeterminate checkboxes are used elsewhere in the app? I can't get this to work consistently and it'd help to compare to a completed implementation.

MisRob commented 2 years ago

Hi @andersan,

I was able to locate some places in code where indeterminate checkboxes are used, however, as a user, I can't really see the desired output as those checkboxes that should be indeterminate are rather empty. I will check on this with the team to find out whether this was an intentional UI change and the code related to the indeterminate state is obsolete or if I should report those occurrences as bugs. Until then, I can't really provide a good working example, so the expected behavior section of this issue is probably the best reference for now,

What are the problems you're experiencing?

MisRob commented 2 years ago

@andersan So we were able to locate a place we can provide as an example after all, even though the use-case is a bit different. If that's helpful, you can find indeterminate checkboxes in action in Device when managing channel resources - for example. select just one resource in a child folder and then navigate one level higher:

indeterminate

andersan commented 2 years ago

Thanks for the example of a place where the indeterminate checkbox is used. I can also see one in the "Create new quiz" UI in the coach area.

I'd like to outline some changes to the behavior in this view (selecting lesson recipients, the focus of this issue thread). That way, we can all be on the same page of what the UX should be.

  1. If a group is selected and nothing else is selected, the individual learner selector interface will not open.
  2. If the entire class is selected by pressing the entire class button, it will not open the individual learners list. (Not sure if this is the right UX - let me know please. I don't know how we'd want the user to open the individual learner's list after selecting the entire class, but that is likely not an important use case.)
  3. If all learners from a group are selected (e.g. by pressing the top "Entire class" checkbox, or by selecting all individual learners from that group), then that checkbox will become checked.
  4. If some learners from a group are selected via the individual learner selector interface, the checkbox for that group will become indeterminate.
  5. If all learners from the class are selected, either by selecting all groups (and there are no un-grouped learners) or selecting all individual learners, the "Entire class" checkbox will become checked.
  6. If some learners are selected via the individual learner selector interface, but not all learners are selected, the "Individual learners" checkbox will be indeterminate.

Please let me know if the functionality above is correct, or if it should be different.

I think there are a lot of specific cases which can become just as confusing as the current UI if this feature is not implemented correctly.

In any case, there is a lot going on in this selector file. I'll need to do some more digging as to how to implement this if the above UI is indeed the correct one. For example, I'm not sure how to check if the list of individual selected learners includes all learners from a class and use that information to update the "Entire class" checkbox. I can send more specific technical questions after the UX for the cases listed above are clear.

Tagging @marcellamaki here as you mentioned MisRob is on vacation.

marcellamaki commented 2 years ago

Sorry for the delay @andersan and thanks for following up on this.

In regards to your questions:

  1. If all learners from a group are selected (e.g. by pressing the top "Entire class" checkbox, or by selecting all individual learners from that group), then that checkbox will become checked. - Correct
  2. If some learners from a group are selected via the individual learner selector interface, the checkbox for that group will become indeterminate. - Correct
  3. If all learners from the class are selected, either by selecting all groups (and there are no un-grouped learners) or selecting all individual learners, the "Entire class" checkbox will become checked. - Correct
  4. If some learners are selected via the individual learner selector interface, but not all learners are selected, the "Individual learners" checkbox will be indeterminate. - Correct

I'm actually going to ask @rtibbles and/or @jtamiace to verify about numbers 1 and 2

  1. If a group is selected and nothing else is selected, the individual learner selector interface will not open.
  2. If the entire class is selected by pressing the entire class button, it will not open the individual learners list. (Not sure if this is the right UX - let me know please. I don't know how we'd want the user to open the individual learner's list after selecting the entire class, but that is likely not an important use case.)

I'm not 100% sure and I want to be certain we give you the right direction. I think that we may want these selectors to open, so that individuals could be de-selected (the use case I am thinking about is, in a class or class of 30, it might be much more efficient to select all, and then deselect 3, rather than select 27, but to your point, I'm not sure how critical that situation is).

Thanks for your patience, and your continued contributions! Someone will get back to you about the first two as soon as wel can.

Akila-I commented 1 year ago

Hi @andersan , are you still working on this issue? If not i'd like to take over.

andersan commented 1 year ago

Hello, please feel free to take over here

On Mon, Feb 27, 2023 at 12:13 PM Akila Induranga Gamage < @.***> wrote:

Hi @andersan https://github.com/andersan , are you still working on this issue? If not i'd like to take over.

— Reply to this email directly, view it on GitHub https://github.com/learningequality/kolibri/issues/9097#issuecomment-1446719996, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBYR4JV3OFNU7XLNYGWVLDWZTODPANCNFSM5OBBLMVQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Chris Anderson (517) 599-9228 he/him/his

Akila-I commented 1 year ago

I got another issue #10124 assigned to me, I'll try to work on this when its done :)

shrinishant commented 1 year ago

Hey @Akila-I, are you working on this? else i'd like to work on this

Akila-I commented 1 year ago

Hi @shrinishant, No I'm not working on this right now. Feel free to get yourself assigned and try on this.

vaibhav-1508 commented 1 year ago

@MisRob Hey there! I'm looking forward to contribute but there aren't any unassigned good first issues as of now😅 Can you please guide me? Also, is there a Slack community that I can join?

manish3299 commented 1 year ago

Hey, I think I can solve this issue, @bjester can I take this forward? Please assign this issue to me.

MisRob commented 1 year ago

Hello @vaibhav-1508 and @manish3299, thank you for your interest. Looking at the comments, it seems there are some open questions about expected behavior, so it'd recommend finding another issue.