cengage / react-magma

https://react-magma.cengage.com
MIT License
21 stars 13 forks source link

TreeView Issues from verification #1490

Closed silvalaura closed 1 month ago

silvalaura commented 1 month ago

TreeView tickets: #1305, #1286, #1433, #1420, #1431

  1. When Selectable is off, expanding a child closes it and the parent: click to expand 0.1, then 0.2. notice they both close
  2. isDisabled prop requires a full refresh for it to take. Do we care?
  3. checkParent / checkChildren is not being respected when using preselected items. -- in the image below, I expect Mesopotamia children to be checked image
  4. clicking Select All or Clear All does not ignore the disabled items: image image
  5. When I open the CS for Update Selected Items From Outside, there are many warnings. We should also get rid of these in our storybook (TreeView.stories.tsx) but most importantly in CS as our adopters likely copy/paste our examples: -- Property 'checkedStatus' does not exist on type 'never' -- 'api' is possibly 'undefined' -- Property 'itemId' does not exist on type 'never'. -- FolderIcon, ArticleIcon are not used
  6. On the Complex example and on multi select, it reads "[object Object]" on items that are just items and not parents on both VoiceOver and NVDA image
  7. We can clean up the isDisabled prop description on useTreeItem, and delete the following lines:
    * TODO: improve functionality (issue #1305)
    * @internal

    There's also some unneeded internal lines on line 40

    /**
    * @internal
    */
chris-cedrone-cengage commented 1 week ago

Heading, IndeterminateCheckboxStatus, TagSize, & TagColor references are missing from Docs example Selectable

chris-cedrone-cengage commented 1 week ago

1305:

General

A user can have a disabled item that is also part of the preselectedItems list. This item cannot be manually deselected by clicking on it, and it also can't be indirectly deselected through the TreeViewApi A user can pass a disabled item to initialExpandedItems

Selectable Off:

If a parent item is disabled: -- the item is visibly disabled ✅ -- all its children are disabled ✅ -- it should not be expandable by clicking or through keyboard commands ✅ If a child item is disabled -- the item is visibly disabled ✅ Keyboard navigation skips over this item ✅

Selectable Single:

If a parent item is disabled: -- the item is visibly disabled ✅ -- all its children are disabled ✅ -- it should not be expandable by clicking or through keyboard commands ✅ -- it should not be selectable by clicking or through keyboard commands ✅ If a child item is disabled -- the item is visibly disabled ✅ -- it should not be selectable by clicking or through keyboard commands ✅ Keyboard navigation skips over this item ✅ An item can be selected and disabled (through preselectedItems) ✅

Selectable Multi:

If a parent item is disabled: -- the item is visibly disabled ✅ -- all its children are disabled when checkChildren is true ✅ -- all its children are unaffected when checkChildren is false ✅ -- it should not be expandable by clicking or through keyboard commands ✅ -- it should not be selectable by clicking or through keyboard commands ✅ If a child item is disabled -- the item is visibly disabled ✅ -- it should not be selectable by clicking or through keyboard commands ✅ -- all its parents are disabled when checkParents is true ✅ -- all its parents are unaffected when checkParents is false ✅ Keyboard navigation skips over this item An item can be selected and disabled (through preselectedItems) ✅ When TreeApi.selectAll gets called, this item is not selected ✅

chris-cedrone-cengage commented 1 week ago

1286:

If a child item is deselected, all the parents need to update ✅ If a parent item is deselected, all the children item need to update ✅ If a parent AND child item is deselected, both the parents and children need to update ✅ If an item is collapsed and selected, it should update appropriately ✅ If an item is expanded and selected, it should update appropriately ✅ onSelectedChange should continue to only get called once ✅ Should work with and without preselectedItems prop ✅

Create an example where tags get added to the bottom of the tree once they are selected (like https://storybook-preview-dev--upbeat-sinoussi-f675aa.netlify.app/?path=/story/treeview--first-item-branch). Then, the user can click on any of those tags and the item gets deselected from the tree and removed from the tags. ✅

-- (when checkParents and checkChildren are set to true) If the item is a child or parent, the parent or child tags get affected and should also be deselected from tree and removed from tag list. ✅

-- (when checkParents and checkChildren are set to false) Only that item is deselected from tree and removed from tag list. ✅

Create an example for TreeViewSelectable.multi with "Select All" and "Deselect All" buttons below the tree ✅

chris-cedrone-cengage commented 1 week ago

1433:

This example should have 10 tags, including pt1ch1.1. ✅

We should also confirm this is behaving as expected with different combinations of preselectedItems, some parents, some children, etc. ✅

chris-cedrone-cengage commented 1 week ago

1420:

Verified by @silvalaura:

Test in either NVDA or JAWS to see if arrow key navigation works without any manual mode changes. ✅

chris-cedrone-cengage commented 1 week ago

1431:

If a nested item is passed to initialExpandedItems, all its parents are also expanded ✅

chris-cedrone-cengage commented 1 week ago

All tickets verified on:

https://storybook-preview-dev--upbeat-sinoussi-f675aa.netlify.app/?path=/story/treeview--complex&args=initialExpandedItems[1]:LOVE+ME

&

https://next--upbeat-sinoussi-f675aa.netlify.app/api/tree-view/#preselected_items