rusackas commented on a change in pull request #14184:
URL: https://github.com/apache/superset/pull/14184#discussion_r624318575
##########
File path: superset-frontend/src/components/Menu/Menu.tsx
##########
@@ -151,147 +258,116 @@ const StyledHeader = styled.header`
${({ theme }) => theme.gridUnit * 2}px;
}
}
+ .ant-menu-submenu-popup {
+ border-radius: 0px !important;
Review comment:
Check to see if we need all the !importants in these files.
##########
File path: superset-frontend/src/components/Menu/Menu.tsx
##########
@@ -92,16 +105,108 @@ const StyledHeader = styled.header`
flex-direction: column;
justify-content: center;
}
-
- .nav > li > a {
+ @media (max-width: 767px) {
+ .navbar-brand {
+ float: none;
+ }
+ }
+ .ant-menu-horizontal .ant-menu-item {
+ height: 100%;
+ line-height: inherit;
+ }
+ .ant-menu > .ant-menu-item > a {
padding: ${({ theme }) => theme.gridUnit * 4}px;
}
+ @media (max-width: 767px) {
+ .ant-menu > .ant-menu-item > a {
+ padding: 0px;
+ }
+ .main-nav .ant-menu-submenu-title > svg:nth-child(1) {
+ display: none;
+ }
+ .ant-menu-item-active > a {
+ &:hover {
+ color: ${({ theme }) => theme.colors.primary.base} !important;
+ background-color: transparent !important;
+ }
+ }
+ }
.dropdown-header {
text-transform: uppercase;
padding-left: 12px;
}
+ .ant-menu-horizontal:not(.ant-menu-dark) > .ant-menu-submenu,
+ .ant-menu-horizontal:not(.ant-menu-dark) > .ant-menu-item {
+ margin: 0px;
+ &:hover {
+ border-bottom: none;
+ }
+ }
+ .ant-menu-horizontal > .ant-menu-item,
+ .ant-menu-horizontal > .ant-menu-submenu {
+ vertical-align: inherit;
+ &:hover {
+ color: ${({ theme }) => theme.colors.grayscale.dark1};
+ }
+ }
+ .navbar-right {
+ border: none;
+ }
+ .ant-menu-horizontal {
+ line-height: 51px;
+ border: none;
+ .ant-menu-submenu-open,
+ .ant-menu-submenu-active,
+ .ant-menu-submenu,
+ .ant-menu-submenu-horizontal {
+ color: ${({ theme }) => theme.colors.grayscale.dark1};
+ border-bottom: none;
+ }
- .navbar-inverse .navbar-nav li a {
+ .ant-menu-submenu-open,
+ .ant-menu-submenu-active {
+ background-color: ${({ theme }) => theme.colors.primary.light5};
+ .ant-menu-submenu-title {
+ color: ${({ theme }) => theme.colors.grayscale.dark1};
+ background-color: ${({ theme }) => theme.colors.primary.light5};
+ border-bottom: none;
+ margin: 0;
+ &:after {
+ opacity: 1;
+ width: 99%;
Review comment:
Check to see if this is needed... normally I'd expect to see 100% or
something like calc(100% - 1px);
##########
File path: superset-frontend/src/components/Menu/Menu.tsx
##########
@@ -151,147 +258,116 @@ const StyledHeader = styled.header`
${({ theme }) => theme.gridUnit * 2}px;
}
}
+ .ant-menu-submenu-popup {
+ border-radius: 0px !important;
+ background-color: red !important;
+ }
`;
+const { SubMenu } = DropdownMenu;
+
export function Menu({
data: { menu, brand, navbar_right: navbarRight, settings },
isFrontendRoute = () => false,
}: MenuProps) {
- const [dropdownOpen, setDropdownOpen] = useState(false);
+ const [showMenu, setMenu] = useState<MenuMode>('horizontal');
- return (
- <StyledHeader className="top" id="main-menu">
- <Navbar inverse fluid staticTop role="navigation">
- <Navbar.Header>
- <Navbar.Brand>
- <a className="navbar-brand" href={brand.path}>
- <img width={brand.width} src={brand.icon} alt={brand.alt} />
- </a>
- </Navbar.Brand>
- <Navbar.Toggle />
- </Navbar.Header>
- <Nav data-test="navbar-top">
- {menu.map((item, index) => {
- const props = {
- ...item,
- isFrontendRoute: isFrontendRoute(item.url),
- childs: item.childs?.map(c => {
- if (typeof c === 'string') {
- return c;
- }
+ useEffect(() => {
+ function handleResize() {
+ if (window.innerWidth <= 767) {
+ setMenu('inline');
+ } else setMenu('horizontal');
+ }
+ handleResize();
Review comment:
or, check out `useResizeDetector` from the Explore chart panel
##########
File path: superset-frontend/src/components/Menu/Menu.tsx
##########
@@ -92,16 +105,108 @@ const StyledHeader = styled.header`
flex-direction: column;
justify-content: center;
}
-
- .nav > li > a {
+ @media (max-width: 767px) {
+ .navbar-brand {
+ float: none;
+ }
+ }
+ .ant-menu-horizontal .ant-menu-item {
+ height: 100%;
+ line-height: inherit;
+ }
+ .ant-menu > .ant-menu-item > a {
padding: ${({ theme }) => theme.gridUnit * 4}px;
}
+ @media (max-width: 767px) {
+ .ant-menu > .ant-menu-item > a {
+ padding: 0px;
+ }
+ .main-nav .ant-menu-submenu-title > svg:nth-child(1) {
+ display: none;
+ }
+ .ant-menu-item-active > a {
+ &:hover {
+ color: ${({ theme }) => theme.colors.primary.base} !important;
+ background-color: transparent !important;
+ }
+ }
+ }
.dropdown-header {
text-transform: uppercase;
padding-left: 12px;
}
+ .ant-menu-horizontal:not(.ant-menu-dark) > .ant-menu-submenu,
+ .ant-menu-horizontal:not(.ant-menu-dark) > .ant-menu-item {
+ margin: 0px;
+ &:hover {
+ border-bottom: none;
+ }
+ }
+ .ant-menu-horizontal > .ant-menu-item,
+ .ant-menu-horizontal > .ant-menu-submenu {
+ vertical-align: inherit;
+ &:hover {
+ color: ${({ theme }) => theme.colors.grayscale.dark1};
+ }
+ }
+ .navbar-right {
+ border: none;
+ }
+ .ant-menu-horizontal {
+ line-height: 51px;
Review comment:
If this is for vertical centering, _maybe_ we can do something with
flexbox to vertically center the content
##########
File path: superset-frontend/src/components/Menu/Menu.tsx
##########
@@ -151,147 +258,116 @@ const StyledHeader = styled.header`
${({ theme }) => theme.gridUnit * 2}px;
}
}
+ .ant-menu-submenu-popup {
+ border-radius: 0px !important;
+ background-color: red !important;
+ }
`;
+const { SubMenu } = DropdownMenu;
+
export function Menu({
data: { menu, brand, navbar_right: navbarRight, settings },
isFrontendRoute = () => false,
}: MenuProps) {
- const [dropdownOpen, setDropdownOpen] = useState(false);
+ const [showMenu, setMenu] = useState<MenuMode>('horizontal');
- return (
- <StyledHeader className="top" id="main-menu">
- <Navbar inverse fluid staticTop role="navigation">
- <Navbar.Header>
- <Navbar.Brand>
- <a className="navbar-brand" href={brand.path}>
- <img width={brand.width} src={brand.icon} alt={brand.alt} />
- </a>
- </Navbar.Brand>
- <Navbar.Toggle />
- </Navbar.Header>
- <Nav data-test="navbar-top">
- {menu.map((item, index) => {
- const props = {
- ...item,
- isFrontendRoute: isFrontendRoute(item.url),
- childs: item.childs?.map(c => {
- if (typeof c === 'string') {
- return c;
- }
+ useEffect(() => {
+ function handleResize() {
+ if (window.innerWidth <= 767) {
+ setMenu('inline');
+ } else setMenu('horizontal');
+ }
+ handleResize();
Review comment:
debounce this... maybe with lodash, or @suddjian might have something to
leverage...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
