Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: move data nav menu to plus menu #18629

Merged
merged 21 commits into from
Feb 17, 2022
Merged

chore: move data nav menu to plus menu #18629

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
681c20e
more data nav menu
pkdotson Feb 9, 2022
a4bbd5e
fix lint and fix nav css
pkdotson Feb 9, 2022
2ae8696
update test and remove icons
pkdotson Feb 9, 2022
237263a
Update superset-frontend/src/views/components/Menu.test.tsx
pkdotson Feb 10, 2022
04d1ff9
Apply suggestions from code review
hughhhh Feb 10, 2022
1c0f383
use backend app.link to show new nav changes
pkdotson Feb 10, 2022
760d8c6
Merge branch 'chore-change-nav' of https://github.com/preset-io/super…
pkdotson Feb 10, 2022
717c5bb
fix lint
pkdotson Feb 10, 2022
2502a9a
update test
pkdotson Feb 10, 2022
123eff6
usetheme and remove chaining
pkdotson Feb 11, 2022
785f74b
add more suggestions
pkdotson Feb 14, 2022
757d502
fix lint
pkdotson Feb 14, 2022
a6c0bbe
add allowed extensions to bootstrap and hard code links
pkdotson Feb 15, 2022
0e9abe9
remove backend links
pkdotson Feb 15, 2022
117261e
fix test
pkdotson Feb 15, 2022
c42491d
add extensions to frontend conf
pkdotson Feb 16, 2022
9eac285
fix test and add be changes
pkdotson Feb 16, 2022
6e9eb0c
test is python test passes
pkdotson Feb 16, 2022
9121e01
update python test and reremove app links
pkdotson Feb 16, 2022
32f6e79
Merge branch 'master' of https://github.com/preset-io/superset into c…
pkdotson Feb 16, 2022
57e001c
fix ts and add t's
pkdotson Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion superset-frontend/src/views/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ import { RootContextProviders } from './RootContextProviders';
setupApp();

const user = { ...bootstrapData.user };
const menu = { ...bootstrapData.common.menu_data };
const menu = {
...bootstrapData.common.menu_data,
allowedExtensions: bootstrapData.common.allowed_extensions,
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
};
let lastLocationPathname: string;
initFeatureFlags(bootstrapData.common.feature_flags);

Expand Down
53 changes: 48 additions & 5 deletions superset-frontend/src/views/components/Menu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,51 @@ const mockedProps = {
url: '/dashboard/list/',
index: 4,
},
{
name: 'Data',
icon: 'fa-database',
label: 'Data',
childs: [
{
name: 'Databases',
icon: 'fa-database',
label: 'Databases',
url: '/databaseview/list/',
},
{
name: 'Datasets',
icon: 'fa-table',
label: 'Datasets',
url: '/tablemodelview/list/',
},
'-',
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
],
},
{
name: 'Upload Data',
icon: 'fa-database',
label: 'Data',
childs: [
{
name: 'Upload a CSV',
icon: 'fa-upload',
label: 'Upload a CSV',
url: '/csvtodatabaseview/form',
},
{
name: 'Upload a Columnar file',
icon: 'fa-upload',
label: 'Upload a Columnar File',
url: '/columnartodatabaseview/form',
},
{
name: 'Upload Excel',
icon: 'fa-upload',
label: 'Upload Excel',
url: '/exceltodatabaseview/form',
},
],
},
],
brand: {
path: '/superset/profile/admin/',
Expand Down Expand Up @@ -220,13 +265,11 @@ test('should render the dropdown items', async () => {
render(<Menu {...notanonProps} />);
const dropdown = screen.getByTestId('new-dropdown-icon');
userEvent.hover(dropdown);
expect(await screen.findByText(dropdownItems[0].label)).toHaveAttribute(
// todo (philip): test data submenu
expect(await screen.findByText(dropdownItems[1].label)).toHaveAttribute(
'href',
dropdownItems[0].url,
dropdownItems[1].url,
);
expect(
screen.getByTestId(`menu-item-${dropdownItems[0].label}`),
).toBeInTheDocument();
expect(await screen.findByText(dropdownItems[1].label)).toHaveAttribute(
'href',
dropdownItems[1].url,
Expand Down
47 changes: 30 additions & 17 deletions superset-frontend/src/views/components/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import React, { useState, useEffect } from 'react';
import { styled, css } from '@superset-ui/core';
import { styled, css, useTheme, SupersetTheme } from '@superset-ui/core';
import { debounce } from 'lodash';
import { Global } from '@emotion/react';
import { getUrlParam } from 'src/utils/urlUtils';
Expand Down Expand Up @@ -68,15 +68,20 @@ export interface MenuProps {
brand: BrandProps;
navbar_right: NavBarProps;
settings: MenuObjectProps[];
allowedExtensions: {
columnar_extensions: boolean;
csv_extensions: boolean;
excel_extensions: boolean;
};
};
isFrontendRoute?: (path?: string) => boolean;
}

interface MenuObjectChildProps {
label: string;
name?: string;
icon: string;
index: number;
name: string;
icon?: string;
index?: number;
url?: string;
isFrontendRoute?: boolean;
}
Expand Down Expand Up @@ -172,18 +177,34 @@ const StyledHeader = styled.header`
}
}
`;

const globalStyles = (theme: SupersetTheme) => css`
.ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light.ant-menu-submenu-placement-bottomLeft {
border-radius: 0px;
}
.ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light {
border-radius: 0px;
}
.ant-menu-vertical > .ant-menu-submenu.data-menu > .ant-menu-submenu-title {
height: 28px;
i {
padding-right: ${theme.gridUnit * 2}px;
margin-left: ${theme.gridUnit * 1.75}px;
}
}
`;
const { SubMenu } = DropdownMenu;

const { useBreakpoint } = Grid;

export function Menu({
data: { menu, brand, navbar_right: navbarRight, settings },
data: { menu, brand, navbar_right: navbarRight, settings, allowedExtensions },
isFrontendRoute = () => false,
}: MenuProps) {
const [showMenu, setMenu] = useState<MenuMode>('horizontal');
const screens = useBreakpoint();
const uiConig = useUiConfig();
const theme = useTheme();
const filteredMenu = menu.filter(item => item.name !== 'Upload Data');
pkdotson marked this conversation as resolved.
Show resolved Hide resolved

useEffect(() => {
function handleResize() {
Expand Down Expand Up @@ -251,16 +272,7 @@ export function Menu({
};
return (
<StyledHeader className="top" id="main-menu" role="navigation">
<Global
styles={css`
.ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light.ant-menu-submenu-placement-bottomLeft {
border-radius: 0px;
}
.ant-menu-submenu.ant-menu-submenu-popup.ant-menu.ant-menu-light {
border-radius: 0px;
}
`}
/>
<Global styles={globalStyles(theme)} />
<Row>
<Col md={16} xs={24}>
<Tooltip
Expand All @@ -283,7 +295,7 @@ export function Menu({
data-test="navbar-top"
className="main-nav"
>
{menu.map(item => {
{filteredMenu.map(item => {
const props = {
...item,
isFrontendRoute: isFrontendRoute(item.url),
Expand All @@ -305,6 +317,7 @@ export function Menu({
</Col>
<Col md={8} xs={24}>
<RightMenu
allowedExtensions={allowedExtensions}
align={screens.md ? 'flex-end' : 'flex-start'}
settings={settings}
navbarRight={navbarRight}
Expand Down
80 changes: 74 additions & 6 deletions superset-frontend/src/views/components/MenuRight.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,30 @@ import LanguagePicker from './LanguagePicker';
import { NavBarProps, MenuObjectProps } from './Menu';

export const dropdownItems = [
{
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
label: t('Data'),
icon: 'fa-database',
childs: [
{
icon: 'fa-upload',
label: 'Upload a CSV',
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
name: 'Upload a CSV',
url: '/csvtodatabaseview/form',
},
{
icon: 'fa-upload',
label: 'Upload a Columnar File',
name: 'Upload a Columnar file',
url: '/columnartodatabaseview/form',
},
{
icon: 'fa-upload',
label: 'Upload Excel',
name: 'Upload Excel',
url: '/exceltodatabaseview/form',
},
],
},
{
label: t('SQL query'),
url: '/superset/sqllab?new=true',
Expand Down Expand Up @@ -85,23 +109,34 @@ interface RightMenuProps {
settings: MenuObjectProps[];
navbarRight: NavBarProps;
isFrontendRoute: (path?: string) => boolean;
allowedExtensions: {
columnar_extensions: boolean;
csv_extensions: boolean;
excel_extensions: boolean;
};
}

const RightMenu = ({
align,
settings,
navbarRight,
isFrontendRoute,
allowedExtensions,
}: RightMenuProps) => {
const { roles } = useSelector<any, UserWithPermissionsAndRoles>(
state => state.user,
);

// if user has any of these roles the dropdown will appear
const canSql = findPermission('can_sqllab', 'Superset', roles);
const canDashboard = findPermission('can_write', 'Dashboard', roles);
const canChart = findPermission('can_write', 'Chart', roles);
const showActionDropdown = canSql || canChart || canDashboard;
const menuIconAndLabel = (menu: MenuObjectProps) => (
<>
<i data-test={`menu-item-${menu.label}`} className={`fa ${menu.icon}`} />
{menu.label}
</>
);
return (
<StyledDiv align={align}>
<Menu mode="horizontal">
Expand All @@ -113,9 +148,41 @@ const RightMenu = ({
}
icon={<Icons.TriangleDown />}
>
{dropdownItems.map(
menu =>
findPermission(menu.perm, menu.view, roles) && (
{dropdownItems.map(menu => {
if (menu.label === 'Data') {
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
return (
<SubMenu
key="sub2"
className="data-menu"
title={menuIconAndLabel(menu as MenuObjectProps)}
>
{allowedExtensions?.csv_extensions && (
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
<Menu.Item key="Upload a CSV">
<a href="/csvtodatabaseview/form"> Upload a CSV </a>
</Menu.Item>
)}
{allowedExtensions?.columnar_extensions && (
<Menu.Item key="Upload a Columnar File">
<a href="/columnartodatabaseview/form">
{' '}
Upload a Columnar File{' '}
</a>
</Menu.Item>
)}
{allowedExtensions?.excel_extensions && (
<Menu.Item key="Upload Excel">
<a href="/exceltodatabaseview/form"> Upload Excel </a>
</Menu.Item>
)}
</SubMenu>
);
}
return (
findPermission(
menu.perm as string,
menu.view as string,
roles,
) && (
<Menu.Item key={menu.label}>
<a href={menu.url}>
<i
Expand All @@ -125,8 +192,9 @@ const RightMenu = ({
{menu.label}
</a>
</Menu.Item>
),
)}
)
);
})}
</SubMenu>
)}
<SubMenu
Expand Down
47 changes: 0 additions & 47 deletions superset/initialization/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,53 +367,6 @@ def init_views(self) -> None:
category_icon="fa-table",
)
appbuilder.add_separator("Data")
appbuilder.add_link(
"Upload a CSV",
label=__("Upload a CSV"),
href="/csvtodatabaseview/form",
icon="fa-upload",
category="Data",
category_label=__("Data"),
category_icon="fa-wrench",
cond=lambda: bool(
self.config["CSV_EXTENSIONS"].intersection(
self.config["ALLOWED_EXTENSIONS"]
)
),
)
appbuilder.add_link(
"Upload a Columnar file",
label=__("Upload a Columnar File"),
href="/columnartodatabaseview/form",
icon="fa-upload",
category="Data",
category_label=__("Data"),
category_icon="fa-wrench",
cond=lambda: bool(
self.config["COLUMNAR_EXTENSIONS"].intersection(
self.config["ALLOWED_EXTENSIONS"]
)
),
)
try:
import xlrd # pylint: disable=unused-import

appbuilder.add_link(
"Upload Excel",
label=__("Upload Excel"),
href="/exceltodatabaseview/form",
icon="fa-upload",
category="Data",
category_label=__("Data"),
category_icon="fa-wrench",
cond=lambda: bool(
self.config["EXCEL_EXTENSIONS"].intersection(
self.config["ALLOWED_EXTENSIONS"]
)
),
)
except ImportError:
pass

appbuilder.add_api(LogRestApi)
appbuilder.add_view(
Expand Down
11 changes: 11 additions & 0 deletions superset/views/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,17 @@ def common_bootstrap_payload() -> Dict[str, Any]:
"extra_sequential_color_schemes": conf["EXTRA_SEQUENTIAL_COLOR_SCHEMES"],
"extra_categorical_color_schemes": conf["EXTRA_CATEGORICAL_COLOR_SCHEMES"],
"theme_overrides": conf["THEME_OVERRIDES"],
"allowed_extensions": {
"excel_extensions": bool(
conf["EXCEL_EXTENSIONS"].intersection(conf["ALLOWED_EXTENSIONS"])
),
"csv_extensions": bool(
conf["CSV_EXTENSIONS"].intersection(conf["ALLOWED_EXTENSIONS"])
),
"columnar_extensions": bool(
conf["COLUMNAR_EXTENSIONS"].intersection(conf["ALLOWED_EXTENSIONS"])
),
},
pkdotson marked this conversation as resolved.
Show resolved Hide resolved
"menu_data": menu_data(),
}
bootstrap_data.update(conf["COMMON_BOOTSTRAP_OVERRIDES_FUNC"](bootstrap_data))
Expand Down