[GitHub] [incubator-superset] kgabryje commented on a change in pull request #11528: refactor: Use Antd Menu in Menu component

[GitBox <gi...@apache.org>]

kgabryje commented on a change in pull request #11528:
URL: https://github.com/apache/incubator-superset/pull/11528#discussion_r516199961



##########
File path: superset-frontend/spec/javascripts/components/Menu_spec.jsx
##########
@@ -18,7 +18,8 @@
  */
 import React from 'react';
 import { shallow, mount } from 'enzyme';
-import { Nav, MenuItem } from 'react-bootstrap';
+import { Nav } from 'react-bootstrap';
+import { Menu as DropdownMenu } from 'src/common/components';

Review comment:
       The main component is called `Menu` and the names clashed

##########
File path: superset-frontend/src/common/components/index.tsx
##########
@@ -43,6 +43,11 @@ export const MenuItem = styled(AntdMenu.Item)`
   > a {
     text-decoration: none;
   }
+
+  &.ant-menu-item {
+    height: 30px;
+    line-height: 30px;
+  }

Review comment:
       I _think_ I did that to increase specificity... I'll check again if that works without `&.ant-menu-item`.
   Good point on using `theme.typography.sizes.m * 2`, will change

##########
File path: superset-frontend/src/common/components/index.tsx
##########
@@ -43,6 +43,11 @@ export const MenuItem = styled(AntdMenu.Item)`
   > a {
     text-decoration: none;
   }
+
+  &.ant-menu-item {
+    height: 30px;
+    line-height: 30px;
+  }

Review comment:
       Done.
   Also, I verified that `&.ant-menu-item` syntax is necessary - if we don't use it, antd's styles have higher specificity.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org