fix: Consolidating dropdown/NavDropdown user experience (removing React-bootstrap, using AntD)

superset-frontend/src/components/EditableTitle/index.tsx

@@ -110,12 +110,9 @@ export default function EditableTitle({
     }
   }
 
-  // this entire method exists to support using EditableTitle as the title of a
-  // react-bootstrap Tab, as a workaround for this line in react-bootstrap https://goo.gl/ZVLmv4
-  //
-  // tl;dr when a Tab EditableTitle is being edited, typically the Tab it's within has been
-  // clicked and is focused/active. for accessibility, when focused the Tab <a /> intercepts
-  // the ' ' key (among others, including all arrows) and onChange() doesn't fire. somehow
+  // tl;dr when a EditableTitle is being edited, typically the Tab that wraps it has been
+  // clicked and is focused/active. For accessibility, when the focused tab anchor intercepts
+  // the ' ' key (among others, including all arrows) the onChange() doesn't fire. Somehow
   // keydown is still called so we can detect this and manually add a ' ' to the current title
   function handleKeyDown(event: any) {
     if (event.key === ' ') {

superset-frontend/src/components/Menu/LanguagePicker.test.tsx

@@ -18,6 +18,7 @@
  */
 import React from 'react';
 import { render, screen } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
 import LanguagePicker from './LanguagePicker';
 
 const mockedProps = {
@@ -41,14 +42,14 @@ test('should render', () => {
   expect(container).toBeInTheDocument();
 });
 
-test('should render the button', () => {
+test('should render the combobox', () => {
   render(<LanguagePicker {...mockedProps} />);
-  const button = screen.getByRole('button');
-  expect(button).toHaveAttribute('href', '#');
+  expect(screen.getByRole('combobox')).toBeInTheDocument();
 });
 
-test('should render the menuitem', () => {
+test('should render the items', async () => {
   render(<LanguagePicker {...mockedProps} />);
-  const menuitem = screen.getByRole('menuitem');
-  expect(menuitem).toHaveTextContent('Italian');
+  userEvent.click(screen.getByRole('combobox'));
+  expect(await screen.findByText('English')).toBeInTheDocument();
+  expect(await screen.findByText('Italian')).toBeInTheDocument();
 });

superset-frontend/src/components/Menu/LanguagePicker.tsx

@@ -17,8 +17,9 @@
  * under the License.
  */
 import React, { useState } from 'react';
-import { Menu } from 'src/common/components';
-import NavDropdown from 'src/components/NavDropdown';
+import { Select } from 'src/common/components';
+import { styled, useTheme } from '@superset-ui/core';
+import Icons from 'src/components/Icons';
 
 export interface Languages {
   [key: string]: {
@@ -33,43 +34,81 @@ interface LanguagePickerProps {
   languages: Languages;
 }
 
+const dropdownWidth = 150;
+
+const StyledLabel = styled.div`
+  display: flex;
+  align-items: center;
+
+  & i {
+    margin-right: ${({ theme }) => theme.gridUnit}px;
+  }
+
+  & span {
+    display: block;
+    width: ${dropdownWidth}px;
+    word-wrap: break-word;
+    white-space: normal;
+  }
+`;
+
+const StyledFlag = styled.div`
+  margin-top: 2px;
+`;
+
+const StyledIcon = styled(Icons.TriangleDown)`
+  ${({ theme }) => `
+    margin-top: -${theme.gridUnit}px;
+    margin-left: -${theme.gridUnit * 2}px;
+  `}
+`;
+
 export default function LanguagePicker({
   locale,
   languages,
 }: LanguagePickerProps) {
-  const [dropdownOpen, setDropdownOpen] = useState(false);
+  const theme = useTheme();
+  const [open, setOpen] = useState(false);
+
+  const options = Object.keys(languages).map(langKey => ({
+    label: (
+      <StyledLabel className="f16">
+        <i className={`flag ${languages[langKey].flag}`} />{' '}
+        <span>{languages[langKey].name}</span>
+      </StyledLabel>
+    ),
+    value: langKey,
+    flag: (
+      <StyledFlag className="f16">
+        <i className={`flag ${languages[langKey].flag}`} />
+      </StyledFlag>
+    ),
+  }));
 
   return (
-    <NavDropdown
-      onMouseEnter={() => setDropdownOpen(true)}
-      onMouseLeave={() => setDropdownOpen(false)}
-      onToggle={value => setDropdownOpen(value)}
-      open={dropdownOpen}
-      id="locale-dropdown"
-      title={
-        <span className="f16">
-          <i className={`flag ${languages[locale].flag}`} />
-        </span>
+    <Select
+      defaultValue={locale}
+      open={open}
+      onMouseEnter={() => setOpen(true)}
+      onMouseLeave={() => setOpen(false)}
+      onDropdownVisibleChange={open => setOpen(open)}
+      bordered={false}
+      options={options}
+      suffixIcon={
+        <StyledIcon
+          iconColor={theme.colors.grayscale.base}
+          className="ant-select-suffix"
+        />
       }
-      data-test="language-picker"
-    >
-      <Menu
-        onSelect={({ key }) => {
-          window.location.href = languages[key].url;
-        }}
-      >
-        {Object.keys(languages).map(langKey =>
-          langKey === locale ? null : (
-            <Menu.Item key={langKey}>
-              {' '}
-              <div className="f16">
-                <i className={`flag ${languages[langKey].flag}`} /> -{' '}
-                {languages[langKey].name}
-              </div>
-            </Menu.Item>
-          ),
-        )}
-      </Menu>
-    </NavDropdown>
+      listHeight={400}
+      dropdownAlign={{
+        offset: [-dropdownWidth, 0],
+      }}
+      optionLabelProp="flag"
+      dropdownMatchSelectWidth={false}
+      onChange={(value: string) => {
+        window.location.href = languages[value].url;
+      }}
+    />
   );
 }

superset-frontend/src/components/Menu/Menu.test.tsx

@@ -303,7 +303,7 @@ test('should render the Documentation link when available', async () => {
   render(<Menu {...mockedProps} />);
   userEvent.hover(screen.getByText('Settings'));
   const doc = await screen.findByTitle('Documentation');
-  expect(doc.firstChild).toHaveAttribute('href', documentation_url);
+  expect(doc).toHaveAttribute('href', documentation_url);
 });
 
 test('should render the Bug Report link when available', async () => {
@@ -314,8 +314,8 @@ test('should render the Bug Report link when available', async () => {
   } = mockedProps;
 
   render(<Menu {...mockedProps} />);
-  const bugReport = await screen.findByTitle('Report a Bug');
-  expect(bugReport.firstChild).toHaveAttribute('href', bug_report_url);
+  const bugReport = await screen.findByTitle('Report a bug');
+  expect(bugReport).toHaveAttribute('href', bug_report_url);
 });
 
 test('should render the Login link when user is anonymous', () => {
@@ -332,5 +332,5 @@ test('should render the Login link when user is anonymous', () => {
 
 test('should render the Language Picker', () => {
   render(<Menu {...mockedProps} />);
-  expect(screen.getByTestId('language-picker')).toBeInTheDocument();
+  expect(screen.getByRole('combobox')).toBeInTheDocument();
 });

superset-frontend/src/components/Menu/Menu.tsx

@@ -201,7 +201,7 @@ export function Menu({
   return (
     <StyledHeader className="top" id="main-menu" role="navigation">
       <Row>
-        <Col lg={19} md={19} sm={24} xs={24}>
+        <Col lg={12} md={24} sm={24} xs={24}>
           <a className="navbar-brand" href={brand.path}>
             <img width={brand.width} src={brand.icon} alt={brand.alt} />
           </a>
@@ -210,7 +210,7 @@ export function Menu({
             data-test="navbar-top"
             className="main-nav"
           >
-            {menu.map((item, index) => {
+            {menu.map(item => {
               const props = {
                 ...item,
                 isFrontendRoute: isFrontendRoute(item.url),
@@ -230,7 +230,7 @@ export function Menu({
             })}
           </DropdownMenu>
         </Col>
-        <Col lg={5} md={5} sm={24} xs={24}>
+        <Col lg={12} md={24} sm={24} xs={24}>
           <RightMenu
             settings={settings}
             navbarRight={navbarRight}