From d8b5d6d21fce1a0bc716f8957f18e0c854b13ed5 Mon Sep 17 00:00:00 2001 From: Tanner Summers Date: Wed, 8 Feb 2023 13:03:29 -0600 Subject: [PATCH 1/2] test(TreeView): updated treeview test to use rtl (#13025) * test(TreeView): updated test to use rtl * test(TreeView): updated test * Update packages/react/src/components/TreeView/TreeView-test.js Co-authored-by: Taylor Jones * Apply suggestions from code review Co-authored-by: Taylor Jones * test(treeview): updated test --------- Co-authored-by: TJ Egan Co-authored-by: Taylor Jones Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../src/components/TreeView/TreeView-test.js | 247 +++-- .../__snapshots__/TreeView-test.js.snap | 990 ------------------ 2 files changed, 179 insertions(+), 1058 deletions(-) delete mode 100644 packages/react/src/components/TreeView/__snapshots__/TreeView-test.js.snap diff --git a/packages/react/src/components/TreeView/TreeView-test.js b/packages/react/src/components/TreeView/TreeView-test.js index 265b33d15a77..974bed95a802 100644 --- a/packages/react/src/components/TreeView/TreeView-test.js +++ b/packages/react/src/components/TreeView/TreeView-test.js @@ -1,95 +1,206 @@ /** - * Copyright IBM Corp. 2020 + * Copyright IBM Corp. 2022 * * This source code is licensed under the Apache-2.0 license found in the * LICENSE file in the root directory of this source tree. */ -import { CaretDown } from '@carbon/icons-react'; import React from 'react'; -import { mount } from 'enzyme'; -import { Document, Folder } from '@carbon/icons-react'; -import TreeView, { TreeNode } from './'; +import TreeView from './TreeView'; +import TreeNode from './TreeNode'; +import userEvent from '@testing-library/user-event'; +import { fireEvent, render, screen, within } from '@testing-library/react'; const prefix = 'cds'; describe('TreeView', () => { - let wrapper; - let onTreeSelect; - let onNodeSelect; - - beforeEach(() => { - onTreeSelect = jest.fn(); - onNodeSelect = jest.fn(); - wrapper = mount( - - - - - - - - + describe('renders as expected - Component API', () => { + it('should spread extra props onto outermost element', () => { + render(); + + expect(screen.getByRole('tree')).toHaveAttribute( + 'data-testid', + 'test-id' + ); + }); + + it('should respect active prop', () => { + render( + + + + + ); + + const node = screen.getByTestId('Node 1'); + + expect(node).toHaveClass(`${prefix}--tree-node--active`); + }); + + it('should render children as expected', () => { + render( + + + - - - ); - }); + + ); - it('should render', () => { - expect(wrapper).toMatchSnapshot(); - }); + const nodeParent = screen.getByTestId('Node 1'); + const nodeChild = screen.getByTestId('Node 2'); + + expect(nodeParent).toHaveClass(`${prefix}--tree-parent-node`); + expect(nodeChild).toHaveClass(`${prefix}--tree-leaf-node`); + + expect(within(nodeParent).getByText('Node 1')).toBeInTheDocument(); + expect(within(nodeChild).getByText('Node 2')).toBeInTheDocument(); + }); + + it('should support a custom `className` prop on the outermost element', () => { + const { container } = render( + + ); + + const ul = container?.getElementsByTagName('ul')[0]; + + expect(ul).toHaveClass('custom-class'); + }); + + it('should respect hideLabel prop', () => { + render( + + + + ); + + expect( + screen.queryByText('Tree View', { selector: 'label' }) + ).not.toBeInTheDocument(); + }); + + it('should respect label prop', () => { + render( + + + + ); + + expect(screen.getByLabelText('Tree View')).toBeInTheDocument(); + }); + + it('should respect multiselect prop', () => { + render( + + + + + ); + + const tree = screen.getByRole('tree'); + const lists = tree?.getElementsByTagName('li'); - it('should render with icons', () => { - wrapper = mount( - - - - - - + userEvent.click(lists[0], { ctrlKey: true }); + userEvent.click(lists[1], { ctrlKey: true }); + + expect(lists[0]).toHaveAttribute('aria-selected', 'true'); + expect(lists[1]).toHaveAttribute('aria-selected', 'true'); + }); + + it('should respect onSelect prop', () => { + const onSelectSpy = jest.fn(); + + render( + - - - - - ); - expect(wrapper).toMatchSnapshot(); - }); + onSelect={onSelectSpy} + id="Node 1" + data-testid="Node 1" + label="Node 1" + /> + + ); - it('should be able to hide the label', () => { - expect(wrapper.find('TreeLabel').text()).toBe('Tree view'); - wrapper.setProps({ hideLabel: true }); - expect(wrapper.find('TreeLabel').text()).toBeFalsy(); - }); + fireEvent.click(screen.getByTestId('Node 1')); - describe('Single node selection', () => { - it('should be able to preselect a node', () => { - expect(wrapper.find(`.${prefix}--tree-node--selected`).text()).toBe('1'); + expect(onSelectSpy).toHaveBeenCalled(); }); - it('should handle selection at the tree level', () => { - const onTreeSelect = jest.fn(); - wrapper.setProps({ onSelect: onTreeSelect }); - wrapper.find('TreeNode[value="2"]').simulate('click'); - expect(onTreeSelect).toHaveBeenCalledTimes(1); + it('should respect selected prop', () => { + render( + + + + ); + + expect(screen.getByRole('treeitem', { name: 'Node 1' })).toHaveClass( + `${prefix}--tree-node--selected` + ); }); - it('should handle selection at the node level', () => { - wrapper.find('TreeNode[value="2"]').simulate('click'); - expect(onTreeSelect).toHaveBeenCalledTimes(1); - expect(onNodeSelect).toHaveBeenCalledTimes(1); + it('should respect size prop', () => { + render(); + + const tree = screen.getByRole('tree'); + + expect(tree).toHaveClass(`${prefix}--tree--xs`); }); }); - describe('Tree node expansion', () => { - it('Caret icon should not render in leaf nodes', () => { - expect(wrapper.find(CaretDown).length).toBe(2); + describe('behaves as expected', () => { + it('should render tree with expanded node', () => { + render( + + + + + + ); + + const nodeParent = screen.getByTestId('Node 1'); + const nodeChild = nodeParent?.querySelector('div > span'); + + expect(nodeChild).toHaveClass(`${prefix}--tree-parent-node__toggle`); + expect(within(nodeParent).getByText('Node 1')).toBeInTheDocument(); + expect(within(nodeParent).getByText('Node 2')).toBeInTheDocument(); + }); + + it('should render tree with disabled nodes', () => { + render( + + + + + + + ); + + const nodeChild = screen.getByTestId('Node 2'); + + expect(nodeChild.getAttribute('aria-disabled')).toEqual('true'); + }); + + it('should render tree with icons', () => { + const CustomIcon = jest.fn(() => { + return ; + }); + + render( + + + + ); + + expect(screen.getByTestId('Node 1')).toHaveClass( + `${prefix}--tree-node--with-icon` + ); }); }); }); diff --git a/packages/react/src/components/TreeView/__snapshots__/TreeView-test.js.snap b/packages/react/src/components/TreeView/__snapshots__/TreeView-test.js.snap deleted file mode 100644 index c81ef376d6db..000000000000 --- a/packages/react/src/components/TreeView/__snapshots__/TreeView-test.js.snap +++ /dev/null @@ -1,990 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`TreeView should render 1`] = ` - - - - -
    - -
  • -
    - 1 -
    -
    • -     - -
  • -
    - 2 -
    -
    • -     - -
  • -
    - - - - - - - - - - - 5 - -
    -
      - -
    • -
      - 5-1 -
      -
      • -       - -
    • -
      - 5-2 -
      -
      • -       - -
    • -
      - - - - - - - - - - - 5-3 - -
      -
        - -
      • -
        - 5-4 -
        -
        • -         -
      -
      • -       -
    -
    • -     -
- -`; - -exports[`TreeView should render with icons 1`] = ` - - - - -
    - -
  • -
    - - - - - - - - - 1 -
    -
    • -     - -
  • -
    - - - - - - - - - 2 -
    -
    • -     - -
  • -
    - - - - - - - - - - - - - - - - - - 5 - -
    -
      - -
    • -
      - - - - - - - - - 5-1 -
      -
      • -       - -
    • -
      - - - - - - - - - 5-2 -
      -
      • -       - -
    • -
      - - - - - - - - - - - - - - - - - - 5-3 - -
      -
        - -
      • -
        - - - - - - - - - 5-4 -
        -
        • -         -
      -
      • -       -
    -
    • -     -
- -`; From e328f22690a04ec08c29d26b31c9f945c3809adb Mon Sep 17 00:00:00 2001 From: TJ Egan Date: Wed, 8 Feb 2023 14:31:53 -0500 Subject: [PATCH 2/2] fix(Toggletip): general a11y / ux improvements (#13112) * fix(Toggletip): general a11y / ux improvements * style(ToogletipLabel): scope type changes to non-form labels --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../components/Toggletip/Toggletip.stories.js | 69 ++++++++++++++----- .../react/src/components/Toggletip/index.js | 25 +++++++ .../styles/scss/components/form/_form.scss | 4 ++ .../scss/components/toggletip/_toggletip.scss | 2 +- 4 files changed, 80 insertions(+), 20 deletions(-) diff --git a/packages/react/src/components/Toggletip/Toggletip.stories.js b/packages/react/src/components/Toggletip/Toggletip.stories.js index 4079e31c815c..05fdab936afc 100644 --- a/packages/react/src/components/Toggletip/Toggletip.stories.js +++ b/packages/react/src/components/Toggletip/Toggletip.stories.js @@ -51,23 +51,53 @@ export default { export const Default = () => { return ( -
- Toggletip label - - - - - -

- Lorem ipsum dolor sit amet, di os consectetur adipiscing elit, sed - do eiusmod tempor incididunt ut fsil labore et dolore magna aliqua. -

- - Link action - - - - +
+
+ Toggletip label + + + + + +

+ Lorem ipsum dolor sit amet, di os consectetur adipiscing elit, sed + do eiusmod tempor incididunt ut fsil labore et dolore magna + aliqua. +

+ + Link action + + + + +
+
+
+
+ + Toggletip label -- using defaultOpen prop + + + + + + +

+ Lorem ipsum dolor sit amet, di os consectetur adipiscing elit, sed + do eiusmod tempor incididunt ut fsil labore et dolore magna + aliqua. +

+ + Link action + + + + +
); }; @@ -76,7 +106,9 @@ const PlaygroundStory = (controls) => { const { align } = controls; return ( <> - Toggletip label + + Toggletip label -- using defaultOpen prop + @@ -130,7 +162,6 @@ Playground.story = { style={{ display: 'flex', alignItems: 'center', - justifyContent: 'center', }}> {story()}
diff --git a/packages/react/src/components/Toggletip/index.js b/packages/react/src/components/Toggletip/index.js index 7f0985540ee8..d3fe1714d744 100644 --- a/packages/react/src/components/Toggletip/index.js +++ b/packages/react/src/components/Toggletip/index.js @@ -95,9 +95,33 @@ function Toggletip({ function onKeyDown(event) { if (open && match(event, keys.Escape)) { actions.close(); + + // If the menu is closed while focus is still inside the menu, it should return to the trigger button (#12922) + const button = ref.current.children[0]; + if (button && button.type === 'button') { + button.focus(); + } + } + } + + function handleBlur(event) { + // Do not close if the menu itself is clicked, should only close on focus out + if (open && event.relatedTarget === null) { + return; + } + if (!event.currentTarget.contains(event.relatedTarget)) { + // The menu should be closed when focus leaves the `Toggletip` (#12922) + actions.close(); } } + // If the `Toggletip` is the last focusable item in the tab order, it shoudl also close when the browser window loses focus (#12922) + useWindowEvent('blur', () => { + if (open) { + actions.close(); + } + }); + useWindowEvent('click', (event) => { if (open && !ref.current.contains(event.target)) { actions.close(); @@ -115,6 +139,7 @@ function Toggletip({ highContrast open={open} onKeyDown={onKeyDown} + onBlur={handleBlur} ref={ref}> {children} diff --git a/packages/styles/scss/components/form/_form.scss b/packages/styles/scss/components/form/_form.scss index 468f51611383..2f360a1eb283 100644 --- a/packages/styles/scss/components/form/_form.scss +++ b/packages/styles/scss/components/form/_form.scss @@ -59,6 +59,10 @@ $input-label-weight: 400 !default; vertical-align: baseline; } + .#{$prefix}--label .#{$prefix}--toggletip-label { + @include type-style('label-01'); + } + .#{$prefix}--label--no-margin { margin-bottom: 0; } diff --git a/packages/styles/scss/components/toggletip/_toggletip.scss b/packages/styles/scss/components/toggletip/_toggletip.scss index 1ccd573d9759..e69342388a9c 100644 --- a/packages/styles/scss/components/toggletip/_toggletip.scss +++ b/packages/styles/scss/components/toggletip/_toggletip.scss @@ -16,7 +16,7 @@ @mixin toggletip() { .#{$prefix}--toggletip-label { - @include type.type-style('label-01'); + @include type.type-style('body-01'); margin-right: spacing.$spacing-03; color: theme.$text-secondary;