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

Checkbox tree fixes #760

Merged
merged 14 commits into from
Jun 17, 2022
Merged

Checkbox tree fixes #760

merged 14 commits into from
Jun 17, 2022

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Jun 14, 2022

Closes #743

Code Changes

  • Icon fixes for arrow up and arrow down icons
  • Expanded state which is separate from selected state
  • Indeterminate state when not all children of a parent are selected
  • Checking a child makes parent "checked" (or indeterminate) but not "selected"
  • When a parent is checked, all of its children are also checked, but disabled

Steps to Confirm

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

There are a lot of UX things baked into this checkbox for our use case, so the easiest way to see if it is working as expected is probably just to play with it yourself. Hopefully the tests cover all of the cases too.

Screen.Recording.2022-06-14.at.1.46.52.PM.mov

@allisonking allisonking marked this pull request as ready for review June 14, 2022 19:32
@allisonking allisonking requested a review from a team June 14, 2022 19:32
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! One small UX nit

@ThomasLaPiana
Copy link
Contributor

@allisonking This is potentially for another issue but something I'd suggest from a UX perspective is that when I click "between" the arrow and the category itself (the dead space) it doesn't do anything. This felt unintuitive, as I'd expect it to either trigger the drop down (first choice) or select the category (second choice). Clicking on it and having nothing happened felt a bit off because the row was still highlighted

@allisonking allisonking requested a review from LKCSmith June 15, 2022 16:38
@allisonking allisonking merged commit 8bdfc26 into main Jun 17, 2022
@allisonking allisonking deleted the aking-743-checkbox-tree-fixes branch June 17, 2022 15:34
ssangervasi added a commit that referenced this pull request Jun 21, 2022
* main:
  Add the UI to the compose deployment (#757)
  [579][748]/manual entry & save/cancel progress in config wizard (#772)
  Remove wip features (#783)
  Checkbox tree fixes (#760)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add arrow icons to indicate nested data categories
3 participants