Display Palette category content using keyboard

[srikant-ch5]

Ref: #1713

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

[tomlyn]

Hi @srikant-ch5 Looks like there is a failure in one of the Cypress test.

[tomlyn]

Hi Srikant
It looks like there is a double flash of the node icons when opening the palette categories multiple times using the keyboard. This doesn't happen when using the mouse/trackpad. Can it be fexed?

[srikant-ch5]

Hi Srikant It looks like there is a double flash of the node icons when opening the palette categories multiple times using the keyboard. This doesn't happen when using the mouse/trackpad. Can it be fexed?

Hi @tomlyn Thanks for the review. I have updated PR with a fix in which the functionality is working fine as expected. As per Cypress test AssertionError: Timed out retrying after 4000ms: Expected to find element: .bx--accordion__item--active, but never found it. but while testing in browser manually when I open a category using space/enter it is showing bx--accordion__item--active. So I am looking into this.

[srikant-ch5]

Hi @tomlyn Please review this PR. I have fixed Cypress test cases which were failing due to code changes.

[matthoward366]

With this being a carbon control shouldn't this already be fully accessible? I wouldn't want to do anything special for this. btw, we'd have this same problem in common-properties.

[srikant-ch5]

With this being a carbon control shouldn't this already be fully accessible? I wouldn't want to do anything special for this. btw, we'd have this same problem in common-properties.

Hi @matthoward366 Accordion implementation in Palette is different as content is dynamically loaded based on Accordion Title Obj Click and as per description here https://github.com/elyra-ai/canvas/blob/bb63e21cffecb6a73d1ba7d72

35416d22a2c06cc/canvas_modules/common-canvas/src/palette/palette-flyout-content-category.jsx#L184 so when we tab and click space action is triggered on AccordionItem and not on above TitleObj .

I have tested tabbing in Common Properties Accordion #1648 and it is working fine.

Screen.Recording.2024-02-28.at.11.19.24.AM.mov

[matthoward366]

Those links don't work for me. Is there a reason they are implemented differently? Should we change something with the implementation rather then workaround the issue?

[srikant-ch5]

Those links don't work for me. Is there a reason they are implemented differently? Should we change something with the implementation rather then workaround the issue?

Hi @matthoward366 here is the link for description regarding implementation

canvas/canvas_modules/common-canvas/src/palette/palette-flyout-content-category.jsx

Line 172 in 9864601

// Returns the content object for the AccordionItem. This is only set to

[matthoward366]

So the issue here is returning null instead of content? If we were to return the content all the time even when close does it work correctly without needing this code? If that worked, could we return an empty div instead of null?

[matthoward366]

@tomlyn It looks like the content is removed from the DOM because of this issue #1125. Would this be fixed if we used uniquifyIDs for the inline icons?

Only rendering the content when open is probably good practice. I'll let @tomlyn decide on the PR review.

[tomlyn]

I've played with this quite a lot today and can't see why the flash of the icons occurs when rendering the open category. I don't think using uniquifyIDs will help because the same thing happens when displaying non-SVG icons. I wonder if its a bug in the Carbon code and, who knows, might be fixed in Carbon v11. Anyway, this fix is reasonable given the way we are calling the Accordion.