From c21b392855ce959a1a00b5df2651fa05f88f48af Mon Sep 17 00:00:00 2001 From: bfoxx Date: Tue, 9 Dec 2025 00:04:57 -0500 Subject: [PATCH 1/6] fix(sidebar-toggle): prevent mousdown event propagation --- .../SidebarToggleButton.js | 34 +++++-- .../__tests__/SidebarToggleButton.test.js | 90 ++++++++++++++----- 2 files changed, 96 insertions(+), 28 deletions(-) diff --git a/src/components/sidebar-toggle-button/SidebarToggleButton.js b/src/components/sidebar-toggle-button/SidebarToggleButton.js index 2d66846097..0be9223aac 100644 --- a/src/components/sidebar-toggle-button/SidebarToggleButton.js +++ b/src/components/sidebar-toggle-button/SidebarToggleButton.js @@ -48,23 +48,45 @@ const SidebarToggleButton = ({ return isOpen ? : ; }; + // Adding this to stop the mousedown event from being propogated up to box-annnotatoins as + // that will cause the active annotation to no longer be active which means that it will not be displayed. + // This causes video annotations not to work properly. + const mouseDownHandler = (event: SyntheticMouseEvent) => { + event.preventDefault(); + event.stopPropagation(); + }; + if (isPreviewModernizationEnabled) { const tooltipPositionModernized = direction === DIRECTION_LEFT ? DIRECTION_RIGHT : DIRECTION_LEFT; return ( - + {/* Workaround to attach BP tooltip to legacy button, remove span when buttons are migrated to BP */} - - - {renderButton()} + + + {renderButton()}x ); } return ( - - + + {renderButton()} diff --git a/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js b/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js index 9cfc881dc8..36f1bc6b25 100644 --- a/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js +++ b/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js @@ -1,35 +1,81 @@ import * as React from 'react'; +import { screen, fireEvent } from '@testing-library/react'; +import { render } from '../../../test-utils/testing-library'; import SidebarToggleButton from '..'; describe('components/sidebar-toggle-button/SidebarToggleButton', () => { - test('should render correctly as open', () => { - const wrapper = mount(); + test.each([true, false])( + 'should render correctly as open if isPreviewModernizationEnabled is %s', + isPreviewModernizationEnabled => { + render(, { + wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, + }); - expect(wrapper).toMatchSnapshot(); - }); + const button = screen.getByRole('button'); + expect(button).toBeInTheDocument(); + expect(button).toHaveClass('bdl-SidebarToggleButton'); + }, + ); - test('should render correctly as closed', () => { - const wrapper = mount(); + test.each([true, false])( + 'should render correctly as left oriented toggle when open if isPreviewModernizationEnabled is %s', + isPreviewModernizationEnabled => { + render(, { + wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, + }); - expect(wrapper).toMatchSnapshot(); - }); + const button = screen.getByRole('button'); + expect(button).toBeInTheDocument(); + expect(button).toHaveClass('bdl-SidebarToggleButton'); + }, + ); - test('should have the proper class when it is collapsed', () => { - const wrapper = mount(); + test.each([true, false])( + 'should render correctly as left oriented toggle when closed if isPreviewModernizationEnabled is %s', + isPreviewModernizationEnabled => { + render(, { + wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, + }); - expect(wrapper.find('PlainButton').hasClass('bdl-is-collapsed')).toBeTruthy(); - }); + const button = screen.getByRole('button'); + expect(button).toBeInTheDocument(); + expect(button).toHaveClass('bdl-SidebarToggleButton'); + }, + ); - test('should render correctly as left oriented toggle when open', () => { - const wrapper = mount(); + test.each([true, false])( + 'should stop the mousedown event from being propogated up to box-annnotations if isPreviewModernizationEnabled is %s', + isPreviewModernizationEnabled => { + const onMouseDown = jest.fn(); + render( + // eslint-disable-next-line jsx-a11y/no-static-element-interactions +
+ +
, + { + wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, + }, + ); + const button = screen.getByRole('button'); + fireEvent.mouseDown(button); + expect(onMouseDown).not.toHaveBeenCalled(); + }, + ); - expect(wrapper).toMatchSnapshot(); - }); - - test('should render correctly as left oriented toggle when closed', () => { - const wrapper = mount(); - - expect(wrapper).toMatchSnapshot(); - }); + test.each([true, false])( + 'should show proper button isPreviewModernizationEnabled is %s', + isPreviewModernizationEnabled => { + render(, { + wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, + }); + const button = screen.getByRole('button'); + expect(button).toHaveClass('bdl-SidebarToggleButton'); + if (isPreviewModernizationEnabled) { + expect(button).toHaveClass('bdl-SidebarToggleButton--modernized'); + } else { + expect(button).not.toHaveClass('bdl-SidebarToggleButton--modernized'); + } + }, + ); }); From cbd726793ea201f1710c7396015d85c9f035bd2b Mon Sep 17 00:00:00 2001 From: bfoxx Date: Tue, 9 Dec 2025 01:08:18 -0500 Subject: [PATCH 2/6] fix(sidebar-toggle): prevent mousdown event propagation --- .../SidebarToggleButton.js | 2 +- .../__tests__/SidebarToggleButton.test.js | 76 ++-- .../SidebarToggleButton.test.js.snap | 389 ------------------ 3 files changed, 36 insertions(+), 431 deletions(-) delete mode 100644 src/components/sidebar-toggle-button/__tests__/__snapshots__/SidebarToggleButton.test.js.snap diff --git a/src/components/sidebar-toggle-button/SidebarToggleButton.js b/src/components/sidebar-toggle-button/SidebarToggleButton.js index 0be9223aac..34060382ad 100644 --- a/src/components/sidebar-toggle-button/SidebarToggleButton.js +++ b/src/components/sidebar-toggle-button/SidebarToggleButton.js @@ -71,7 +71,7 @@ const SidebarToggleButton = ({ type="button" {...rest} > - {renderButton()}x + {renderButton()}
diff --git a/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js b/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js index 36f1bc6b25..7855e4219e 100644 --- a/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js +++ b/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js @@ -5,42 +5,35 @@ import { render } from '../../../test-utils/testing-library'; import SidebarToggleButton from '..'; describe('components/sidebar-toggle-button/SidebarToggleButton', () => { - test.each([true, false])( - 'should render correctly as open if isPreviewModernizationEnabled is %s', - isPreviewModernizationEnabled => { - render(, { - wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, - }); - - const button = screen.getByRole('button'); - expect(button).toBeInTheDocument(); - expect(button).toHaveClass('bdl-SidebarToggleButton'); - }, - ); - - test.each([true, false])( - 'should render correctly as left oriented toggle when open if isPreviewModernizationEnabled is %s', - isPreviewModernizationEnabled => { - render(, { - wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, - }); - - const button = screen.getByRole('button'); - expect(button).toBeInTheDocument(); - expect(button).toHaveClass('bdl-SidebarToggleButton'); - }, - ); - - test.each([true, false])( - 'should render correctly as left oriented toggle when closed if isPreviewModernizationEnabled is %s', - isPreviewModernizationEnabled => { - render(, { - wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, + test.each` + isOpen | direction + ${true} | ${'left'} + ${false} | ${'left'} + ${true} | ${'right'} + ${false} | ${'right'} + `( + 'should render correct button correctly when open is $isOpen and direction is $direction and isPreviewModernizationEnabled is false', + ({ isOpen, direction }) => { + render(, { + wrapperProps: { features: { previewModernization: { enabled: false } } }, }); const button = screen.getByRole('button'); - expect(button).toBeInTheDocument(); - expect(button).toHaveClass('bdl-SidebarToggleButton'); + let iconClassName = ''; + let iconNotDisplayedClassName = ''; + if (direction === 'left') { + iconClassName = isOpen ? 'icon-show' : 'icon-hide'; + iconNotDisplayedClassName = isOpen ? 'icon-hide' : 'icon-show'; + } else { + iconClassName = isOpen ? 'icon-hide' : 'icon-show'; + iconNotDisplayedClassName = isOpen ? 'icon-show' : 'icon-hide'; + } + const icon = button.querySelector(`.${iconClassName}`); + const iconNotDisplayed = button.querySelector(`.${iconNotDisplayedClassName}`); + expect(icon).toBeInTheDocument(); + expect(iconNotDisplayed).not.toBeInTheDocument(); + const isCollapsed = button.classList.contains('bdl-is-collapsed'); + expect(isCollapsed).toBe(!isOpen); }, ); @@ -48,10 +41,11 @@ describe('components/sidebar-toggle-button/SidebarToggleButton', () => { 'should stop the mousedown event from being propogated up to box-annnotations if isPreviewModernizationEnabled is %s', isPreviewModernizationEnabled => { const onMouseDown = jest.fn(); + const onClick = jest.fn(); render( // eslint-disable-next-line jsx-a11y/no-static-element-interactions
- +
, { wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, @@ -64,18 +58,18 @@ describe('components/sidebar-toggle-button/SidebarToggleButton', () => { ); test.each([true, false])( - 'should show proper button isPreviewModernizationEnabled is %s', + 'should show proper button isPreviewModernizationEnabled is %s and click handler should work', isPreviewModernizationEnabled => { - render(, { + const onClick = jest.fn(); + render(, { wrapperProps: { features: { previewModernization: { enabled: isPreviewModernizationEnabled } } }, }); const button = screen.getByRole('button'); expect(button).toHaveClass('bdl-SidebarToggleButton'); - if (isPreviewModernizationEnabled) { - expect(button).toHaveClass('bdl-SidebarToggleButton--modernized'); - } else { - expect(button).not.toHaveClass('bdl-SidebarToggleButton--modernized'); - } + const isModernized = button.classList.contains('bdl-SidebarToggleButton--modernized'); + expect(isModernized).toBe(isPreviewModernizationEnabled); + fireEvent.click(button); + expect(onClick).toHaveBeenCalled(); }, ); }); diff --git a/src/components/sidebar-toggle-button/__tests__/__snapshots__/SidebarToggleButton.test.js.snap b/src/components/sidebar-toggle-button/__tests__/__snapshots__/SidebarToggleButton.test.js.snap deleted file mode 100644 index d25c9f336a..0000000000 --- a/src/components/sidebar-toggle-button/__tests__/__snapshots__/SidebarToggleButton.test.js.snap +++ /dev/null @@ -1,389 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`components/sidebar-toggle-button/SidebarToggleButton should render correctly as closed 1`] = ` - - - - } - classPrefix="tooltip" - constraints={ - [ - { - "attachment": "together", - "to": "window", - }, - ] - } - enabled={false} - renderElementTag="div" - renderElementTo={null} - targetAttachment="middle left" - > - - - - - - - -`; - -exports[`components/sidebar-toggle-button/SidebarToggleButton should render correctly as left oriented toggle when closed 1`] = ` - - - - } - classPrefix="tooltip" - constraints={ - [ - { - "attachment": "together", - "to": "window", - }, - ] - } - enabled={false} - renderElementTag="div" - renderElementTo={null} - targetAttachment="middle right" - > - - - - - - - -`; - -exports[`components/sidebar-toggle-button/SidebarToggleButton should render correctly as left oriented toggle when open 1`] = ` - - - - } - classPrefix="tooltip" - constraints={ - [ - { - "attachment": "together", - "to": "window", - }, - ] - } - enabled={false} - renderElementTag="div" - renderElementTo={null} - targetAttachment="middle right" - > - - - - - - - -`; - -exports[`components/sidebar-toggle-button/SidebarToggleButton should render correctly as open 1`] = ` - - - - } - classPrefix="tooltip" - constraints={ - [ - { - "attachment": "together", - "to": "window", - }, - ] - } - enabled={false} - renderElementTag="div" - renderElementTo={null} - targetAttachment="middle left" - > - - - - - - - -`; From e1fc472c7e6aa2fdb39f3929b38ca09b5e5eedb7 Mon Sep 17 00:00:00 2001 From: bfoxx Date: Tue, 9 Dec 2025 12:10:16 -0500 Subject: [PATCH 3/6] feat(video-annotations): removing preventDefault call --- src/components/sidebar-toggle-button/SidebarToggleButton.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/sidebar-toggle-button/SidebarToggleButton.js b/src/components/sidebar-toggle-button/SidebarToggleButton.js index 34060382ad..02ed567656 100644 --- a/src/components/sidebar-toggle-button/SidebarToggleButton.js +++ b/src/components/sidebar-toggle-button/SidebarToggleButton.js @@ -52,7 +52,6 @@ const SidebarToggleButton = ({ // that will cause the active annotation to no longer be active which means that it will not be displayed. // This causes video annotations not to work properly. const mouseDownHandler = (event: SyntheticMouseEvent) => { - event.preventDefault(); event.stopPropagation(); }; From 15424c2597d1af37f16d4da85ff2d6434e8b7a44 Mon Sep 17 00:00:00 2001 From: bfoxx1906 <99675599+bfoxx1906@users.noreply.github.com> Date: Tue, 9 Dec 2025 12:42:13 -0500 Subject: [PATCH 4/6] Update src/components/sidebar-toggle-button/SidebarToggleButton.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/components/sidebar-toggle-button/SidebarToggleButton.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/sidebar-toggle-button/SidebarToggleButton.js b/src/components/sidebar-toggle-button/SidebarToggleButton.js index 02ed567656..31a26e60f9 100644 --- a/src/components/sidebar-toggle-button/SidebarToggleButton.js +++ b/src/components/sidebar-toggle-button/SidebarToggleButton.js @@ -48,7 +48,7 @@ const SidebarToggleButton = ({ return isOpen ? : ; }; - // Adding this to stop the mousedown event from being propogated up to box-annnotatoins as + // Adding this to stop the mousedown event from being propagated up to box-annotations as // that will cause the active annotation to no longer be active which means that it will not be displayed. // This causes video annotations not to work properly. const mouseDownHandler = (event: SyntheticMouseEvent) => { From 2a75a0897922322d5400e25ab44f65b78e72d1b0 Mon Sep 17 00:00:00 2001 From: bfoxx1906 <99675599+bfoxx1906@users.noreply.github.com> Date: Tue, 9 Dec 2025 12:42:34 -0500 Subject: [PATCH 5/6] Update src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- .../sidebar-toggle-button/__tests__/SidebarToggleButton.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js b/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js index 7855e4219e..e00f10abbd 100644 --- a/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js +++ b/src/components/sidebar-toggle-button/__tests__/SidebarToggleButton.test.js @@ -38,7 +38,7 @@ describe('components/sidebar-toggle-button/SidebarToggleButton', () => { ); test.each([true, false])( - 'should stop the mousedown event from being propogated up to box-annnotations if isPreviewModernizationEnabled is %s', + 'should stop the mousedown event from being propagated up to box-annotations if isPreviewModernizationEnabled is %s', isPreviewModernizationEnabled => { const onMouseDown = jest.fn(); const onClick = jest.fn(); From cd6703680fffaca61af81bb6a72a7b4c437ce5f1 Mon Sep 17 00:00:00 2001 From: bfoxx Date: Wed, 10 Dec 2025 14:10:10 -0500 Subject: [PATCH 6/6] fix(sidebar): addressing pr comments --- src/components/sidebar-toggle-button/SidebarToggleButton.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/sidebar-toggle-button/SidebarToggleButton.js b/src/components/sidebar-toggle-button/SidebarToggleButton.js index 31a26e60f9..c62760b738 100644 --- a/src/components/sidebar-toggle-button/SidebarToggleButton.js +++ b/src/components/sidebar-toggle-button/SidebarToggleButton.js @@ -59,7 +59,7 @@ const SidebarToggleButton = ({ const tooltipPositionModernized = direction === DIRECTION_LEFT ? DIRECTION_RIGHT : DIRECTION_LEFT; return ( - + {/* Workaround to attach BP tooltip to legacy button, remove span when buttons are migrated to BP */} +