Skip to content

Commit

Permalink
removing unneccessary data-shift-focus
Browse files Browse the repository at this point in the history
  • Loading branch information
dpitcock committed Sep 18, 2024
1 parent f71d73d commit a9429f9
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 18 deletions.
20 changes: 11 additions & 9 deletions src/app-layout/__tests__/trigger-button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@ const mockEventBubble = {
relatedTarget: null,
};

const mockEventBubbleWithShiftFocusCloseDrawerButton = {
const mockRelatedTarget = new EventTarget();
const mockEventBubbleWithRelatedTarget = {
...mockEventBubble,
relatedTarget: {
dataset: {
shiftFocus: 'last-opened-toolbar-trigger-button',
},
},
relatedTarget: mockRelatedTarget,
// {
// dataset: {
// shiftFocus: 'last-opened-toolbar-trigger-button',
// },
// },
};

const mockQuerySelector = jest.spyOn(document, 'querySelector');
Expand Down Expand Up @@ -268,7 +270,7 @@ describe('Visual refresh trigger-button (not in appLayoutWidget toolbar)', () =>
expect(getByTestId(mockTestId)).toBeTruthy();
expect(button).toBeTruthy();
expect(document.activeElement).not.toBe(button!.getElement());
(ref.current as any)?.focus(mockEventBubbleWithShiftFocusCloseDrawerButton);
(ref.current as any)?.focus(mockEventBubbleWithRelatedTarget);
expect(document.activeElement).toBe(button!.getElement());
});
});
Expand Down Expand Up @@ -365,7 +367,7 @@ describe('Visual Refresh Toolbar trigger-button', () => {
expect(() => getByText(mockTooltipText)).toThrow();
expect(button).toBeTruthy();
expect(document.activeElement).not.toBe(button!.getElement());
(ref.current as any)?.focus(mockEventBubbleWithShiftFocusCloseDrawerButton);
(ref.current as any)?.focus(mockEventBubbleWithRelatedTarget);
expect(document.activeElement).toBe(button!.getElement());
expect(getByTestId(mockTestId)).toBeTruthy();
expect(wrapper!.findByClassName(toolbarTriggerButtonStyles['trigger-tooltip'])).toBeNull();
Expand Down Expand Up @@ -543,7 +545,7 @@ describe('Visual Refresh Toolbar trigger-button', () => {
).toBe(false);
expect(wrapper!.findByClassName(toolbarTriggerButtonStyles['trigger-tooltip'])).toBeNull();
expect(() => getByText(mockTooltipText)).toThrow();
fireEvent.focus(wrapper!.getElement(), { relatedTarget: new EventTarget() });
fireEvent.focus(wrapper!.getElement(), { relatedTarget: mockRelatedTarget });

expect(getByText(mockTooltipText)).toBeTruthy();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,15 @@ function TriggerButton(
if (isForSplitPanel) {
const breadcrumbsComponent = document.querySelector('nav[data-awsui-discovered-breadcrumbs="true"]');
if (breadcrumbsComponent && relatedTarget && breadcrumbsComponent.contains(relatedTarget)) {
//show tooltip when using keys to navigate to it from the breadcrumbs
//show tooltip when using keys to navigate to it from the breadcrumbs as
//breadcrumbs do not have a data-shift-focus like other drawer triggers
shouldShowTooltip = true;
} else {
shouldShowTooltip = isFromAnotherTrigger;
}
} else if (!isForPreviousActiveDrawer) {
shouldShowTooltip = true; //for keyed navigation inside the toolbar
} else if (isFromAnotherTrigger) {
//needed for safari which doesn't read the relatedTarget when drawer closed via
//drawer close button
shouldShowTooltip = true;

Check warning on line 132 in src/app-layout/visual-refresh-toolbar/toolbar/trigger-button/index.tsx

View check run for this annotation

Codecov / codecov/patch

src/app-layout/visual-refresh-toolbar/toolbar/trigger-button/index.tsx#L132

Added line #L132 was not covered by tests
}
setSupressTooltip(!shouldShowTooltip);
Expand Down
7 changes: 1 addition & 6 deletions src/app-layout/visual-refresh/drawers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,7 @@ function ActiveDrawer() {
}
}}
>
{!isMobile && activeDrawer?.resizable && (
<div className={styles['drawer-slider']} data-shift-focus="drawer-resize-button-handle">
{resizeHandle}
</div>
)}
{!isMobile && activeDrawer?.resizable && <div className={styles['drawer-slider']}>{resizeHandle}</div>}
<div className={styles['drawer-content-container']}>
<div className={styles['drawer-close-button']}>
<InternalButton
Expand All @@ -143,7 +139,6 @@ function ActiveDrawer() {
}}
ref={drawersRefs.close}
variant="icon"
data-shift-focus="last-opened-toolbar-trigger-button"
/>
</div>
{toolsContent && (
Expand Down

0 comments on commit a9429f9

Please sign in to comment.