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

Clipboard indeterminate selection incorrect #4636

Closed
bjester opened this issue Aug 12, 2024 · 10 comments · Fixed by #4691
Closed

Clipboard indeterminate selection incorrect #4636

bjester opened this issue Aug 12, 2024 · 10 comments · Fixed by #4691
Assignees

Comments

@bjester
Copy link
Member

bjester commented Aug 12, 2024

Observed behavior

When a topic or folder has descendants selected, but itself isn't selected, it shows as unselected, with ancestors selected.
image_720

Additionally, there's also a minor cosmetic fix for the alignment of the checkbox, which could be fixed along with this issue.

Expected behavior

A topic/folder should be shown as indeterminately unselected when it has descendants selected, but itself isn't selected.
image_720

User-facing consequences

The indeterminate states are meant to make all possible selection states clear to the user. While this is inherently complex in its nature, it's the appropriate manner for visualizing each state, without overlap.

Additional behavior

Replicate the behavior that currently exists in production. As this is likely a regression in the checkbox itself, the state management is unlikely the issue, so attention should be focused towards producing the desired states with the checkbox. Since we migrated to using KDS checkboxes, the issue might be the lack of support for the indeterminately unselected state.

State Description Screenshot
All selected All items within a channel are selected image
Channel partially selected A subset of a channel's items are selected image
Folder unselected A folder itself is unselected, while its descendants are at least partially selected image
Folder partially selected A folder itself is selected but its descendants are partially selected image

Steps to reproduce the issue

  1. Copy a folder to the clipboard
  2. Open the clipboard
  3. Select a subset of the folder's nodes
  4. Observe the folder is shown as indeterminately selected instead of indeterminately unselected

Usage Details

  • Browser: Chrome
  • URL:
  • Other information that may be relevant: unstable branch
@MisRob
Copy link
Member

MisRob commented Aug 14, 2024

@bjester

the issue might be the lack of support for the indeterminately unselected state

Checkbox supports indeterminate state https://design-system.learningequality.org/kcheckbox/#states

So most likely a bug either in Studio or in KDS.

@bjester
Copy link
Member Author

bjester commented Aug 14, 2024

@MisRob KCheckbox supports 'indeterminate' but not 'indeterminate unselected' from what I can see. The docs also say, in regards to the prop, it "Overrides checked state." As you can see in the 'Folder unselected' state, this indeterminate unselected state is gray, because the folder itself isn't selected. So this usage must not override the checked state for proper rendering.

Looking at the source, you can see that the only indeterminate state will render with the primary color (notBlank).
https://github.com/learningequality/kolibri-design-system/blob/ce9e70750e8fe4067b28923305adfe43f646120b/lib/KCheckbox.vue#L26-L43

@MisRob
Copy link
Member

MisRob commented Aug 14, 2024

@bjester Okay. I didn't understand what you mean by 'indeterminately unselected'. Yes, this would be a new state for KCheckbox.

@MisRob
Copy link
Member

MisRob commented Aug 14, 2024

With that I am not suggesting we add a new gray state yet to KDS. Do we know if this is Studio specific - was gray in the designs or was this Vuetify related? I am a bit suspicious this is just a mismatch between the two libraries, and wonder if it'd make sense to have consistent experience and go with the current pattern that is used elsewhere in our products. If there's clarity in that this was intentional rather than just Vuetify's default or there is an intentional nuance I a missing, no problem. I just don't quite yet get what would be the value for the user and if they would be able to understand blue indeterminate selected vs gray indeterminate unselected if they saw them next to each other. For most of the people I'd guess the nature of the indeterminate is simply somewhat..indeterminate :) Alternatively we could change blue to gray for the current indeterminate that would take effect everywhere. Wouldn't like to speculate too much here though, this sounds rather like something for designers to have final call on anyways.

Generally, we will be occasionally running into situations where KDS migration will cause some visual changes to be talked through.

@bjester
Copy link
Member Author

bjester commented Aug 14, 2024

With the current implementation of KCheckbox, there are a total of 3 states for its visual appearance. When selecting items for bulk copy/import or move into a destination, the clipboard selection allows users to specify the conditional selection of a folders along with nodes, which creates more states than the checkbox currently allows. When the user selects a subset of a folder's nodes, and the checkbox becomes indeterminate, then that matches the design expectation where the documentation says: such as when a subset of a topic is selected.

That feature of the clipboard is an intentional UX decision because it allows the users to maintain the folder organization of nodes during the bulk operation. If the user selects a subset of a folder's nodes and the folder itself, then it's indeterminately selected (blue). If the user instead wishes to omit the folder itself from the operation, then it's indeterminately unselected (gray). You can see these in the 'Additional behavior' I outlined in the issue body. The Vuetify library did support this because it's possible from the implementation perspective of the checkbox. I agree users may not immediately know what this means. I don't know that we can provide the functionality we intend, while upholding indeterminate as a subset of a topic is selected, without a fourth state, because with three, the UX is less clear-- 2 states would have to use the same visual appearance.

@MisRob
Copy link
Member

MisRob commented Aug 15, 2024

Thanks for elaborating @bjester - I think I can understand better and agree it is important to preserve distinguishing them for bulk selections - otherwise the underlying feature of having choice to preserve/not-preserve would not be communicated very clearly. Can't think of anything else either. It sounds like an advanced operation and I guess people using the clipboard frequently will learn the difference over time.

One thing that may help and just a side note at this point, sounds like a good candidate for a nice popup during onboarding tutorial for new users to Studio, shall we ever have that :) Perhaps after it is explored in Kolibri. I imagine it could be handy for more reasons.

@MisRob
Copy link
Member

MisRob commented Aug 15, 2024

I'm digging into this further to understand very well so that it informs upcoming KDS migration work that I sometimes open issues for. I think that to prevent from regression like this, it'll be always good to gather the most unusual/tricky places in Studio and decide if there is something that needs to be projected to KDS. If yes, then we'd update KDS, and only then proceed to the Studio refactor itself. We probably won't plan for everything that the refactoring process will reveal, but perhaps it will help a bit.

@bjester
Copy link
Member Author

bjester commented Aug 15, 2024

@MisRob I agree, there's very likely more we can do to make this clearer to the user. I think the hierarchical selection structure limits us in what we can change. So unless we decide to change that design, I think enhancing the UI's explanation of itself should add great value.

@nucleogenesis
Copy link
Member

I think that we might be able to hack the expected visual behavior with some changes to how the isChecked get/set handlers work and applying some conditional styles for the color of the fill on the checkbox.

Basically, KCheckbox will put a check-mark in the box if isChecked=true, even if indeterminate=true.

Seems like we can handle the four states on the Studio side in the short-term if needed, but I suspect that we can do a non-breaking change to KCheckbox.

To be clear on the expectations here, we have the following checkbox states which need to be covered:

Selected Not Selected
Indeterminate [-] (blue) [-] (gray)
Not... [x] (blue) [ ]

I think we can handle this without any breaking changes assuming that we don't inadvertently pass indeterminate as true along with checked together anywhere and are only using that particular pattern here. If Kolibri is using KCheckbox without making indeterminate and checked mutually exclusive.

@nucleogenesis
Copy link
Member

This will need significant regression testing, but learningequality/kolibri-design-system#744 appears to fix the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants