]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-18779 Fix the dropdown component accessibility issue in the design system
authorKevin Silva <kevin.silva@sonarsource.com>
Wed, 29 Mar 2023 11:24:27 +0000 (13:24 +0200)
committersonartech <sonartech@sonarsource.com>
Thu, 30 Mar 2023 20:03:06 +0000 (20:03 +0000)
server/sonar-web/design-system/src/components/Dropdown.tsx
server/sonar-web/design-system/src/components/DropdownToggler.tsx
server/sonar-web/design-system/src/components/__tests__/Dropdown-test.tsx
server/sonar-web/design-system/src/components/popups.tsx
server/sonar-web/src/main/js/app/components/global-search/GlobalSearch.tsx
server/sonar-web/src/main/js/app/components/nav/component/Menu.tsx
server/sonar-web/src/main/js/app/components/nav/component/branch-like/BranchLikeNavigation.tsx

index f04b595decdb92761de7ba9f494f79f85bd3c0bd..3eccd54e68f76f30a75a070263ce338c77171932 100644 (file)
@@ -46,6 +46,7 @@ interface Props {
   className?: string;
   closeOnClick?: boolean;
   id: string;
+  isPortal?: boolean;
   onOpen?: VoidFunction;
   overlay: React.ReactNode;
   placement?: PopupPlacement;
@@ -80,7 +81,15 @@ export default class Dropdown extends React.PureComponent<Props, State> {
 
   render() {
     const { open } = this.state;
-    const { allowResizing, className, closeOnClick = true, id, size = 'full', zLevel } = this.props;
+    const {
+      allowResizing,
+      className,
+      closeOnClick = true,
+      id,
+      isPortal,
+      size = 'full',
+      zLevel,
+    } = this.props;
     const a11yAttrs: A11yAttrs = {
       'aria-controls': `${id}-dropdown`,
       'aria-expanded': open,
@@ -104,6 +113,7 @@ export default class Dropdown extends React.PureComponent<Props, State> {
         aria-labelledby={`${id}-trigger`}
         className={className}
         id={`${id}-dropdown`}
+        isPortal={isPortal}
         onRequestClose={this.handleClose}
         open={open}
         overlay={
index f46f3dc8456e28bee0f31e060e7e7450f5a11494..c57762c5d02af991b9c09e37e46a4453cc6166ea 100644 (file)
@@ -19,9 +19,9 @@
  */
 import EscKeydownHandler from './EscKeydownHandler';
 import OutsideClickHandler from './OutsideClickHandler';
-import { PortalPopup } from './popups';
+import { Popup } from './popups';
 
-type PopupProps = PortalPopup['props'];
+type PopupProps = Popup['props'];
 
 interface Props extends PopupProps {
   onRequestClose: VoidFunction;
@@ -32,7 +32,7 @@ export default function DropdownToggler(props: Props) {
   const { children, open, onRequestClose, overlay, ...popupProps } = props;
 
   return (
-    <PortalPopup
+    <Popup
       overlay={
         open ? (
           <OutsideClickHandler onClickOutside={onRequestClose}>
@@ -43,6 +43,6 @@ export default function DropdownToggler(props: Props) {
       {...popupProps}
     >
       {children}
-    </PortalPopup>
+    </Popup>
   );
 }
index 52139a0489d9231c50915a8dd5c81fb8959d14bc..6d0da97d61578a525f0a38cd02adb41f0a7700b7 100644 (file)
@@ -22,6 +22,33 @@ import { renderWithRouter } from '../../helpers/testUtils';
 import { ButtonSecondary } from '../buttons';
 import Dropdown, { ActionsDropdown } from '../Dropdown';
 
+describe('Dropdown with Portal Wrapper', () => {
+  it('renders', async () => {
+    const { user } = setupWithChildren();
+    expect(screen.getByRole('button')).toBeInTheDocument();
+
+    await user.click(screen.getByRole('button'));
+    expect(screen.getByRole('menu')).toBeInTheDocument();
+  });
+
+  it('toggles with render prop', async () => {
+    const { user } = setupWithChildren(({ onToggleClick }) => (
+      <ButtonSecondary onClick={onToggleClick} />
+    ));
+
+    await user.click(screen.getByRole('button'));
+    expect(screen.getByRole('menu')).toBeVisible();
+  });
+
+  function setupWithChildren(children?: Dropdown['props']['children']) {
+    return renderWithRouter(
+      <Dropdown id="test-menu" isPortal={true} overlay={<div id="overlay" />}>
+        {children ?? <ButtonSecondary />}
+      </Dropdown>
+    );
+  }
+});
+
 describe('Dropdown', () => {
   it('renders', async () => {
     const { user } = setupWithChildren();
index f2ed594ede7b090dbe14742485da7b053df7f62a..0c24ad06213a168a75cf3e14ba91c5313a15b121 100644 (file)
@@ -28,7 +28,7 @@ import { PopupPlacement, popupPositioning, PopupZLevel } from '../helpers/positi
 import { themeBorder, themeColor, themeContrast, themeShadow } from '../helpers/theme';
 import ClickEventBoundary from './ClickEventBoundary';
 
-interface PopupProps {
+interface PopupBaseProps {
   'aria-labelledby'?: string;
   children?: React.ReactNode;
   className?: string;
@@ -39,7 +39,7 @@ interface PopupProps {
   zLevel?: PopupZLevel;
 }
 
-function PopupBase(props: PopupProps, ref: React.Ref<HTMLDivElement>) {
+function PopupBase(props: PopupBaseProps, ref: React.Ref<HTMLDivElement>) {
   const {
     children,
     className,
@@ -66,11 +66,10 @@ function PopupBase(props: PopupProps, ref: React.Ref<HTMLDivElement>) {
 const PopupWithRef = React.forwardRef(PopupBase);
 PopupWithRef.displayName = 'Popup';
 
-export const Popup = PopupWithRef;
-
-interface PortalPopupProps extends Omit<PopupProps, 'style'> {
+interface PopupProps extends Omit<PopupBaseProps, 'style'> {
   allowResizing?: boolean;
   children: React.ReactNode;
+  isPortal?: boolean;
   overlay: React.ReactNode;
 }
 
@@ -87,12 +86,12 @@ function isMeasured(state: State): state is Measurements {
   return state.height !== undefined;
 }
 
-export class PortalPopup extends React.PureComponent<PortalPopupProps, State> {
+export class Popup extends React.PureComponent<PopupProps, State> {
   mounted = false;
   popupNode = React.createRef<HTMLDivElement>();
   throttledPositionTooltip: () => void;
 
-  constructor(props: PortalPopupProps) {
+  constructor(props: PopupProps) {
     super(props);
     this.state = {};
     this.throttledPositionTooltip = throttle(this.positionPopup, THROTTLE_SCROLL_DELAY);
@@ -104,7 +103,7 @@ export class PortalPopup extends React.PureComponent<PortalPopupProps, State> {
     this.mounted = true;
   }
 
-  componentDidUpdate(prevProps: PortalPopupProps) {
+  componentDidUpdate(prevProps: PopupProps) {
     if (this.props.placement !== prevProps.placement || this.props.overlay !== prevProps.overlay) {
       this.positionPopup();
     }
@@ -179,13 +178,23 @@ export class PortalPopup extends React.PureComponent<PortalPopupProps, State> {
     return (
       <>
         {this.props.children}
-        {this.props.overlay && (
-          <PortalWrapper>
-            <Popup placement={placement} ref={this.popupNode} style={style} {...popupProps}>
+        {this.props.overlay &&
+          (this.props.isPortal ? (
+            <PortalWrapper>
+              <PopupWithRef
+                placement={placement}
+                ref={this.popupNode}
+                style={style}
+                {...popupProps}
+              >
+                {overlay}
+              </PopupWithRef>
+            </PortalWrapper>
+          ) : (
+            <PopupWithRef placement={placement} ref={this.popupNode} style={style} {...popupProps}>
               {overlay}
-            </Popup>
-          </PortalWrapper>
-        )}
+            </PopupWithRef>
+          ))}
       </>
     );
   }
index 1906deb6680b82eaf1636d62ce076993f8a5f68a..605b251614f43afc93bb4b439c8d2a7ae302fb06 100644 (file)
@@ -24,8 +24,8 @@ import {
   InteractiveIcon,
   INTERACTIVE_TOOLTIP_DELAY,
   MenuSearchIcon,
+  Popup,
   PopupZLevel,
-  PortalPopup,
   TextMuted,
 } from 'design-system';
 import { debounce, uniqBy } from 'lodash';
@@ -376,7 +376,7 @@ export class GlobalSearch extends React.PureComponent<Props, State> {
 
     const search = (
       <div role="search" className="sw-min-w-abs-200 sw-max-w-abs-350 sw-w-full">
-        <PortalPopup
+        <Popup
           allowResizing={true}
           overlay={
             open && (
@@ -424,7 +424,7 @@ export class GlobalSearch extends React.PureComponent<Props, State> {
             searchInputAriaLabel={translate('search_verb')}
             clearIconAriaLabel={translate('clear')}
           />
-        </PortalPopup>
+        </Popup>
       </div>
     );
 
index ffbda93552d0818b08d9d4b9eaa6d825b7c52d64..027005f86314fedac321d2d4dafc9fb3d20a031d 100644 (file)
@@ -24,6 +24,7 @@ import {
   Link,
   NavBarTabLink,
   NavBarTabs,
+  PopupZLevel,
 } from 'design-system';
 import * as React from 'react';
 import Tooltip from '../../../../components/controls/Tooltip';
@@ -292,6 +293,7 @@ export class Menu extends React.PureComponent<Props> {
         data-test="administration"
         id="component-navigation-admin"
         size="auto"
+        zLevel={PopupZLevel.Global}
         overlay={adminLinks}
       >
         {({ onToggleClick, open, a11yAttrs }) => (
@@ -594,6 +596,7 @@ export class Menu extends React.PureComponent<Props> {
         data-test="extensions"
         id="component-navigation-more"
         size="auto"
+        zLevel={PopupZLevel.Global}
         overlay={withoutSecurityExtension.map((e) => this.renderExtension(e, false, query))}
       >
         {({ onToggleClick, open, a11yAttrs }) => (
index c163935187bcfd59affca0b9f0133c051e721cd6..b8296bd435545772933be8ac77271675a6509199 100644 (file)
@@ -17,7 +17,7 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  */
-import { ButtonSecondary, PopupPlacement, PopupZLevel, PortalPopup } from 'design-system';
+import { ButtonSecondary, Popup, PopupPlacement, PopupZLevel } from 'design-system';
 import * as React from 'react';
 import EscKeydownHandler from '../../../../../components/controls/EscKeydownHandler';
 import OutsideClickHandler from '../../../../../components/controls/OutsideClickHandler';
@@ -75,7 +75,7 @@ export function BranchLikeNavigation(props: BranchLikeNavigationProps) {
             setIsMenuOpen(false);
           }}
         >
-          <PortalPopup
+          <Popup
             allowResizing={true}
             overlay={
               isMenuOpen && (
@@ -104,7 +104,7 @@ export function BranchLikeNavigation(props: BranchLikeNavigationProps) {
             >
               {currentBranchLikeElement}
             </ButtonSecondary>
-          </PortalPopup>
+          </Popup>
         </OutsideClickHandler>
       </EscKeydownHandler>