You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/02/08 15:08:06 UTC

[GitHub] [superset] michael-s-molina opened a new pull request #13002: refactor: Bootstrap to AntD - DropdownButton

michael-s-molina opened a new pull request #13002:
URL: https://github.com/apache/superset/pull/13002


   ### SUMMARY
   - Migrates Collapse component from Bootstrap to AntD.
   - Aligns dropdown caret with text
   
   See: #10254
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <img width="525" alt="Screen Shot 2021-02-05 at 2 50 34 PM" src="https://user-images.githubusercontent.com/70410625/107237531-97da9f00-6a05-11eb-8cef-8b1c5d2b51da.png">
   <img width="525" alt="Screen Shot 2021-02-05 at 2 50 34 PM" src="https://user-images.githubusercontent.com/70410625/107237548-9e691680-6a05-11eb-9d1d-07869d53fe13.png">
   <img width="384" alt="Screen Shot 2021-02-05 at 2 51 08 PM" src="https://user-images.githubusercontent.com/70410625/107237668-bb9de500-6a05-11eb-9233-19f121b0b1b1.png">
   <img width="508" alt="Screen Shot 2021-02-08 at 10 22 04 AM" src="https://user-images.githubusercontent.com/70410625/107237681-c2c4f300-6a05-11eb-9521-c06df242434f.png">
   <img width="770" alt="Screen Shot 2021-02-08 at 10 20 29 AM" src="https://user-images.githubusercontent.com/70410625/107237720-cd7f8800-6a05-11eb-9f4c-47faa77f777f.png">
   <img width="759" alt="Screen Shot 2021-02-08 at 10 21 16 AM" src="https://user-images.githubusercontent.com/70410625/107237736-d3756900-6a05-11eb-9b00-79544fc0d367.png">
   <img width="274" alt="Screen Shot 2021-02-08 at 10 52 39 AM" src="https://user-images.githubusercontent.com/70410625/107237756-d96b4a00-6a05-11eb-912f-57e304ceeac5.png">
   <img width="264" alt="Screen Shot 2021-02-08 at 10 53 11 AM" src="https://user-images.githubusercontent.com/70410625/107237781-dec89480-6a05-11eb-9eb0-a587b9146e19.png">
   
   @rusackas @junlincc
   
   ### TEST PLAN
   1 - Open any screen that contains a `DropdownButton` (explore, dashboard -> edit, etc.)
   2 - All `DropdownButton` components should have the same theme and behavior
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [x] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


----------------------------------------------------------------
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


[GitHub] [superset] michael-s-molina commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r578434286



##########
File path: superset-frontend/src/explore/components/DisplayQueryButton.jsx
##########
@@ -156,48 +153,47 @@ export const DisplayQueryButton = props => {
 
   const { slice } = props;
   return (
-    <DropdownButton
-      open={menuVisible}
-      noCaret
+    <Dropdown
+      trigger="click"
       data-test="query-dropdown"
-      title={
-        <span>
-          <i className="fa fa-bars" />
-          &nbsp;
-        </span>
-      }
-      bsSize="sm"
-      pullRight
-      id="query"
-      onToggle={setMenuVisible}
-    >
-      <Menu onClick={handleMenuClick} selectable={false}>
-        {slice && (
-          <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
-            {t('Edit properties')}
+      overlay={
+        <Menu onClick={handleMenuClick} selectable={false}>
+          {slice && (
+            <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
+              {t('Edit properties')}
+            </Menu.Item>
+          )}
+          <Menu.Item>
+            <ModalTrigger
+              triggerNode={
+                <span data-test="view-query-menu-item">{t('View query')}</span>
+              }
+              modalTitle={t('View query')}
+              beforeOpen={() => beforeOpen('query')}
+              modalBody={renderQueryModalBody()}
+              responsive
+            />
           </Menu.Item>
-        )}
-        <Menu.Item>
-          <ModalTrigger
-            triggerNode={
-              <span data-test="view-query-menu-item">{t('View query')}</span>
-            }
-            modalTitle={t('View query')}
-            beforeOpen={() => beforeOpen('query')}
-            modalBody={renderQueryModalBody()}
-            responsive
-          />
-        </Menu.Item>
-        {sqlSupported && (
-          <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
-            {t('Run in SQL Lab')}
+          {sqlSupported && (
+            <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
+              {t('Run in SQL Lab')}
+            </Menu.Item>
+          )}
+          <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
+            {t('Download as image')}
           </Menu.Item>
-        )}
-        <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
-          {t('Download as image')}
-        </Menu.Item>
-      </Menu>
-    </DropdownButton>
+        </Menu>
+      }
+    >
+      <div
+        role="button"
+        id="query"
+        tabIndex={0}
+        className="btn btn-default btn-sm"
+      >

Review comment:
       Agree. Separate PR.




----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577891104



##########
File path: superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx
##########
@@ -38,7 +38,9 @@ const defaultProps = {
   menuItems: [],
   isFocused: false,
   shouldFocus: (event, container) =>
-    container && container.contains(event.target),
+    (container && container.contains(event.target)) ||
+    event.target.id === 'menu-item' ||
+    (event.target.parentNode && event.target.parentNode.id === 'menu-item'),

Review comment:
       ```suggestion
       (event.target.parentNode && event.target.parentNode?.id === 'menu-item'),
   ```
   ... and here




----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577890231



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -89,25 +90,31 @@ class PopoverDropdown extends React.PureComponent {
     const { id, value, options, renderButton, renderOption } = this.props;
     const selected = options.find(opt => opt.value === value);
     return (
-      <DropdownButton
+      <Dropdown
         id={id}
-        bsSize="small"
-        title={renderButton(selected)}
-        className="popover-dropdown"
+        trigger="click"
+        overlayStyle={{ zIndex: 3001 }}
+        overlay={
+          <Menu onClick={this.handleSelect}>
+            {options.map(option => (
+              <MenuItem
+                id="menu-item"
+                key={option.value}
+                className={cx('dropdown-item', {
+                  active: option.value === value,
+                })}
+              >
+                {renderOption(option)}
+              </MenuItem>
+            ))}
+          </Menu>
+        }
       >
-        <Menu onClick={this.handleSelect}>
-          {options.map(option => (
-            <MenuItem
-              key={option.value}
-              className={cx('dropdown-item', {
-                active: option.value === value,
-              })}
-            >
-              {renderOption(option)}
-            </MenuItem>
-          ))}
-        </Menu>
-      </DropdownButton>
+        <div role="button" css={{ display: 'flex', alignItems: 'center' }}>
+          {renderButton(selected)}
+          <Icon name="caret-down" css={{ marginTop: 4 }} />

Review comment:
       Not sure if this measurement makes more sense based on gridUnit. ¯\\\_(ツ)_/¯ 




----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577903449



##########
File path: superset-frontend/src/explore/components/DisplayQueryButton.jsx
##########
@@ -156,48 +153,47 @@ export const DisplayQueryButton = props => {
 
   const { slice } = props;
   return (
-    <DropdownButton
-      open={menuVisible}
-      noCaret
+    <Dropdown
+      trigger="click"
       data-test="query-dropdown"
-      title={
-        <span>
-          <i className="fa fa-bars" />
-          &nbsp;
-        </span>
-      }
-      bsSize="sm"
-      pullRight
-      id="query"
-      onToggle={setMenuVisible}
-    >
-      <Menu onClick={handleMenuClick} selectable={false}>
-        {slice && (
-          <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
-            {t('Edit properties')}
+      overlay={
+        <Menu onClick={handleMenuClick} selectable={false}>
+          {slice && (
+            <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
+              {t('Edit properties')}
+            </Menu.Item>
+          )}
+          <Menu.Item>
+            <ModalTrigger
+              triggerNode={
+                <span data-test="view-query-menu-item">{t('View query')}</span>
+              }
+              modalTitle={t('View query')}
+              beforeOpen={() => beforeOpen('query')}
+              modalBody={renderQueryModalBody()}
+              responsive
+            />
           </Menu.Item>
-        )}
-        <Menu.Item>
-          <ModalTrigger
-            triggerNode={
-              <span data-test="view-query-menu-item">{t('View query')}</span>
-            }
-            modalTitle={t('View query')}
-            beforeOpen={() => beforeOpen('query')}
-            modalBody={renderQueryModalBody()}
-            responsive
-          />
-        </Menu.Item>
-        {sqlSupported && (
-          <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
-            {t('Run in SQL Lab')}
+          {sqlSupported && (
+            <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
+              {t('Run in SQL Lab')}
+            </Menu.Item>
+          )}
+          <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
+            {t('Download as image')}
           </Menu.Item>
-        )}
-        <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
-          {t('Download as image')}
-        </Menu.Item>
-      </Menu>
-    </DropdownButton>
+        </Menu>
+      }
+    >
+      <div
+        role="button"
+        id="query"
+        tabIndex={0}
+        className="btn btn-default btn-sm"
+      >

Review comment:
       ~~Maybe we can replace this with the shiny new Button component?~~
   
   ...meh, that should be a separte issue/PR.

##########
File path: superset-frontend/src/explore/components/DisplayQueryButton.jsx
##########
@@ -156,48 +153,47 @@ export const DisplayQueryButton = props => {
 
   const { slice } = props;
   return (
-    <DropdownButton
-      open={menuVisible}
-      noCaret
+    <Dropdown
+      trigger="click"
       data-test="query-dropdown"
-      title={
-        <span>
-          <i className="fa fa-bars" />
-          &nbsp;
-        </span>
-      }
-      bsSize="sm"
-      pullRight
-      id="query"
-      onToggle={setMenuVisible}
-    >
-      <Menu onClick={handleMenuClick} selectable={false}>
-        {slice && (
-          <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
-            {t('Edit properties')}
+      overlay={
+        <Menu onClick={handleMenuClick} selectable={false}>
+          {slice && (
+            <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
+              {t('Edit properties')}
+            </Menu.Item>
+          )}
+          <Menu.Item>
+            <ModalTrigger
+              triggerNode={
+                <span data-test="view-query-menu-item">{t('View query')}</span>
+              }
+              modalTitle={t('View query')}
+              beforeOpen={() => beforeOpen('query')}
+              modalBody={renderQueryModalBody()}
+              responsive
+            />
           </Menu.Item>
-        )}
-        <Menu.Item>
-          <ModalTrigger
-            triggerNode={
-              <span data-test="view-query-menu-item">{t('View query')}</span>
-            }
-            modalTitle={t('View query')}
-            beforeOpen={() => beforeOpen('query')}
-            modalBody={renderQueryModalBody()}
-            responsive
-          />
-        </Menu.Item>
-        {sqlSupported && (
-          <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
-            {t('Run in SQL Lab')}
+          {sqlSupported && (
+            <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
+              {t('Run in SQL Lab')}
+            </Menu.Item>
+          )}
+          <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
+            {t('Download as image')}
           </Menu.Item>
-        )}
-        <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
-          {t('Download as image')}
-        </Menu.Item>
-      </Menu>
-    </DropdownButton>
+        </Menu>
+      }
+    >
+      <div
+        role="button"
+        id="query"
+        tabIndex={0}
+        className="btn btn-default btn-sm"
+      >

Review comment:
       ~~Maybe we can replace this with the shiny new Button component?~~
   
   ...meh, that should be a separate issue/PR.




----------------------------------------------------------------
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


[GitHub] [superset] michael-s-molina commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r578430153



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -78,6 +78,7 @@ const MenuItem = styled(Menu.Item)`
 class PopoverDropdown extends React.PureComponent {

Review comment:
       I agree with the suggestion in a separate PR. I added this component to our tests audit spreadsheet with the following notes: 
   
   > We should move this to the components folder, change it to a functional component with typescript, create tests and storybook




----------------------------------------------------------------
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


[GitHub] [superset] michael-s-molina commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r578423493



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -89,25 +90,31 @@ class PopoverDropdown extends React.PureComponent {
     const { id, value, options, renderButton, renderOption } = this.props;
     const selected = options.find(opt => opt.value === value);
     return (
-      <DropdownButton
+      <Dropdown
         id={id}
-        bsSize="small"
-        title={renderButton(selected)}
-        className="popover-dropdown"
+        trigger="click"
+        overlayStyle={{ zIndex: 3001 }}

Review comment:
       I changed to pull `z-index` from the theme for now but I really like the idea of a z-index registry. We should do that in the future.




----------------------------------------------------------------
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


[GitHub] [superset] michael-s-molina commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r578433961



##########
File path: superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx
##########
@@ -38,7 +38,9 @@ const defaultProps = {
   menuItems: [],
   isFocused: false,
   shouldFocus: (event, container) =>
-    container && container.contains(event.target),
+    (container && container.contains(event.target)) ||

Review comment:
       Great suggestion! Done.

##########
File path: superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx
##########
@@ -38,7 +38,9 @@ const defaultProps = {
   menuItems: [],
   isFocused: false,
   shouldFocus: (event, container) =>
-    container && container.contains(event.target),
+    (container && container.contains(event.target)) ||
+    event.target.id === 'menu-item' ||
+    (event.target.parentNode && event.target.parentNode.id === 'menu-item'),

Review comment:
       Great suggestion! Done.




----------------------------------------------------------------
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


[GitHub] [superset] michael-s-molina commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r578603707



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -89,25 +90,31 @@ class PopoverDropdown extends React.PureComponent {
     const { id, value, options, renderButton, renderOption } = this.props;
     const selected = options.find(opt => opt.value === value);
     return (
-      <DropdownButton
+      <Dropdown
         id={id}
-        bsSize="small"
-        title={renderButton(selected)}
-        className="popover-dropdown"
+        trigger="click"
+        overlayStyle={{ zIndex: 3001 }}
+        overlay={
+          <Menu onClick={this.handleSelect}>
+            {options.map(option => (
+              <MenuItem
+                id="menu-item"
+                key={option.value}
+                className={cx('dropdown-item', {
+                  active: option.value === value,
+                })}
+              >
+                {renderOption(option)}
+              </MenuItem>
+            ))}
+          </Menu>
+        }
       >
-        <Menu onClick={this.handleSelect}>
-          {options.map(option => (
-            <MenuItem
-              key={option.value}
-              className={cx('dropdown-item', {
-                active: option.value === value,
-              })}
-            >
-              {renderOption(option)}
-            </MenuItem>
-          ))}
-        </Menu>
-      </DropdownButton>
+        <div role="button" css={{ display: 'flex', alignItems: 'center' }}>
+          {renderButton(selected)}
+          <Icon name="caret-down" css={{ marginTop: 4 }} />

Review comment:
       I really prefer this detailed review to a superficial one. It's very important to keep our standards intact. 😉 




----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577883280



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -89,25 +90,31 @@ class PopoverDropdown extends React.PureComponent {
     const { id, value, options, renderButton, renderOption } = this.props;
     const selected = options.find(opt => opt.value === value);
     return (
-      <DropdownButton
+      <Dropdown
         id={id}
-        bsSize="small"
-        title={renderButton(selected)}
-        className="popover-dropdown"
+        trigger="click"
+        overlayStyle={{ zIndex: 3001 }}

Review comment:
       I'm a little hesitant about random zIndex values re-appearing in the codebase after making some effort to consolidate them. I see a couple paths forward:
   • Making a z-index registry in the Theme (painful for now, due to the external repo)
   • Exporting a z-index value from the Popover itself, and importing here to reference/increment.
   
   
   
   




----------------------------------------------------------------
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


[GitHub] [superset] rusackas merged pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas merged pull request #13002:
URL: https://github.com/apache/superset/pull/13002


   


----------------------------------------------------------------
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


[GitHub] [superset] codecov-io edited a comment on pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13002:
URL: https://github.com/apache/superset/pull/13002#issuecomment-775288673


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=h1) Report
   > Merging [#13002](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=desc) (5034253) into [master](https://codecov.io/gh/apache/superset/commit/2ce79823dfad61bce6196fcacd56a844f44818c0?el=desc) (2ce7982) will **increase** coverage by `5.36%`.
   > The diff coverage is `70.58%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13002/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #13002      +/-   ##
   ==========================================
   + Coverage   53.06%   58.42%   +5.36%     
   ==========================================
     Files         489      468      -21     
     Lines       17314    15873    -1441     
     Branches     4482     4080     -402     
   ==========================================
   + Hits         9187     9274      +87     
   + Misses       8127     6599    -1528     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `58.42% <70.58%> (+5.36%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `4.16% <0.00%> (-0.09%)` | :arrow_down: |
   | [...end/src/SqlLab/components/RunQueryActionButton.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1J1blF1ZXJ5QWN0aW9uQnV0dG9uLnRzeA==) | `52.77% <ø> (ø)` | |
   | [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `100.00% <ø> (ø)` | |
   | [superset-frontend/src/chart/ChartRenderer.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0UmVuZGVyZXIuanN4) | `75.67% <0.00%> (-1.04%)` | :arrow_down: |
   | [...perset-frontend/src/common/components/Dropdown.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL0Ryb3Bkb3duLnRzeA==) | `50.00% <ø> (ø)` | |
   | [...t-frontend/src/common/components/Tooltip/index.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL1Rvb2x0aXAvaW5kZXgudHN4) | `100.00% <ø> (ø)` | |
   | [...perset-frontend/src/components/AlteredSliceTag.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQWx0ZXJlZFNsaWNlVGFnLmpzeA==) | `92.00% <ø> (ø)` | |
   | [superset-frontend/src/components/CachedLabel.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2FjaGVkTGFiZWwuanN4) | `42.10% <ø> (ø)` | |
   | [...ontend/src/components/CertifiedIconWithTooltip.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2VydGlmaWVkSWNvbldpdGhUb29sdGlwLnRzeA==) | `0.00% <ø> (ø)` | |
   | [...perset-frontend/src/components/CopyToClipboard.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ29weVRvQ2xpcGJvYXJkLmpzeA==) | `57.14% <0.00%> (ø)` | |
   | ... and [147 more](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=footer). Last update [f85497e...5034253](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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


[GitHub] [superset] michael-s-molina commented on pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #13002:
URL: https://github.com/apache/superset/pull/13002#issuecomment-775835971


   > not related, @michael-s-molina Michael, could you double check if we covered the casing change in the main menu dropdown? thanks!
   > <img alt="Screen Shot 2021-02-08 at 10 29 28 PM" width="221" src="https://user-images.githubusercontent.com/67837651/107324561-23cbe580-6a5d-11eb-88a0-702d7c01b32f.png">
   
   @junlincc This text is translated on the server side (Python). This is the part that Kasia was working on. We should discuss how to continue this today in our meeting.


----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r578599776



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -89,25 +90,31 @@ class PopoverDropdown extends React.PureComponent {
     const { id, value, options, renderButton, renderOption } = this.props;
     const selected = options.find(opt => opt.value === value);
     return (
-      <DropdownButton
+      <Dropdown
         id={id}
-        bsSize="small"
-        title={renderButton(selected)}
-        className="popover-dropdown"
+        trigger="click"
+        overlayStyle={{ zIndex: 3001 }}
+        overlay={
+          <Menu onClick={this.handleSelect}>
+            {options.map(option => (
+              <MenuItem
+                id="menu-item"
+                key={option.value}
+                className={cx('dropdown-item', {
+                  active: option.value === value,
+                })}
+              >
+                {renderOption(option)}
+              </MenuItem>
+            ))}
+          </Menu>
+        }
       >
-        <Menu onClick={this.handleSelect}>
-          {options.map(option => (
-            <MenuItem
-              key={option.value}
-              className={cx('dropdown-item', {
-                active: option.value === value,
-              })}
-            >
-              {renderOption(option)}
-            </MenuItem>
-          ))}
-        </Menu>
-      </DropdownButton>
+        <div role="button" css={{ display: 'flex', alignItems: 'center' }}>
+          {renderButton(selected)}
+          <Icon name="caret-down" css={{ marginTop: 4 }} />

Review comment:
       I know... it's an annoying pedantic thing I do on every PR. It's always "use the theme colors" or "use gridUnit" everywhere I go. Sorry to be _that guy_.




----------------------------------------------------------------
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


[GitHub] [superset] michael-s-molina commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r578433680



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -89,25 +90,31 @@ class PopoverDropdown extends React.PureComponent {
     const { id, value, options, renderButton, renderOption } = this.props;
     const selected = options.find(opt => opt.value === value);
     return (
-      <DropdownButton
+      <Dropdown
         id={id}
-        bsSize="small"
-        title={renderButton(selected)}
-        className="popover-dropdown"
+        trigger="click"
+        overlayStyle={{ zIndex: 3001 }}
+        overlay={
+          <Menu onClick={this.handleSelect}>
+            {options.map(option => (
+              <MenuItem
+                id="menu-item"
+                key={option.value}
+                className={cx('dropdown-item', {
+                  active: option.value === value,
+                })}
+              >
+                {renderOption(option)}
+              </MenuItem>
+            ))}
+          </Menu>
+        }
       >
-        <Menu onClick={this.handleSelect}>
-          {options.map(option => (
-            <MenuItem
-              key={option.value}
-              className={cx('dropdown-item', {
-                active: option.value === value,
-              })}
-            >
-              {renderOption(option)}
-            </MenuItem>
-          ))}
-        </Menu>
-      </DropdownButton>
+        <div role="button" css={{ display: 'flex', alignItems: 'center' }}>
+          {renderButton(selected)}
+          <Icon name="caret-down" css={{ marginTop: 4 }} />

Review comment:
       Not this one 😆. 
   
   The funny thing is that every time I encounter a multiple of 4 your image comes to my mind with the words "**grid unit**" 🤣 




----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577883280



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -89,25 +90,31 @@ class PopoverDropdown extends React.PureComponent {
     const { id, value, options, renderButton, renderOption } = this.props;
     const selected = options.find(opt => opt.value === value);
     return (
-      <DropdownButton
+      <Dropdown
         id={id}
-        bsSize="small"
-        title={renderButton(selected)}
-        className="popover-dropdown"
+        trigger="click"
+        overlayStyle={{ zIndex: 3001 }}

Review comment:
       I'm a little hesitant about random zIndex values re-appearing in the codebase after making some effort to consolidate them. I see a couple paths forward:
   • Making a z-index registry in the Theme (painful for now, due to the external repo)
   • Exporting a z-index value from the Popover itself, and importing here to reference/increment.
   
   ... there might be other ways too, just to keep collisions/codependencies from arising. Thoughts?
   




----------------------------------------------------------------
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


[GitHub] [superset] codecov-io commented on pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #13002:
URL: https://github.com/apache/superset/pull/13002#issuecomment-775288673


   # [Codecov](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=h1) Report
   > Merging [#13002](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=desc) (b6ee14a) into [master](https://codecov.io/gh/apache/superset/commit/50fa10054fd5d2c28553c4ffd754e12bdedef5da?el=desc) (50fa100) will **increase** coverage by `9.19%`.
   > The diff coverage is `55.55%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/13002/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #13002      +/-   ##
   ==========================================
   + Coverage   52.64%   61.84%   +9.19%     
   ==========================================
     Files         480      545      +65     
     Lines       17300    20137    +2837     
     Branches     4481     5267     +786     
   ==========================================
   + Hits         9108    12454    +3346     
   + Misses       8192     7470     -722     
   - Partials        0      213     +213     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `61.84% <55.55%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [.../src/dashboard/components/menu/WithPopoverMenu.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL21lbnUvV2l0aFBvcG92ZXJNZW51LmpzeA==) | `74.50% <33.33%> (-7.13%)` | :arrow_down: |
   | [...tend/src/explore/components/DisplayQueryButton.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EaXNwbGF5UXVlcnlCdXR0b24uanN4) | `45.16% <60.00%> (-31.41%)` | :arrow_down: |
   | [.../src/dashboard/components/menu/PopoverDropdown.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL21lbnUvUG9wb3ZlckRyb3Bkb3duLmpzeA==) | `68.18% <100.00%> (-22.73%)` | :arrow_down: |
   | [superset-frontend/src/views/App.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0FwcC50c3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/views/menu.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL21lbnUudHN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/views/index.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2luZGV4LnRzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/SqlLab/index.tsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9pbmRleC50c3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [467 more](https://codecov.io/gh/apache/superset/pull/13002/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=footer). Last update [50fa100...b6ee14a](https://codecov.io/gh/apache/superset/pull/13002?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577903449



##########
File path: superset-frontend/src/explore/components/DisplayQueryButton.jsx
##########
@@ -156,48 +153,47 @@ export const DisplayQueryButton = props => {
 
   const { slice } = props;
   return (
-    <DropdownButton
-      open={menuVisible}
-      noCaret
+    <Dropdown
+      trigger="click"
       data-test="query-dropdown"
-      title={
-        <span>
-          <i className="fa fa-bars" />
-          &nbsp;
-        </span>
-      }
-      bsSize="sm"
-      pullRight
-      id="query"
-      onToggle={setMenuVisible}
-    >
-      <Menu onClick={handleMenuClick} selectable={false}>
-        {slice && (
-          <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
-            {t('Edit properties')}
+      overlay={
+        <Menu onClick={handleMenuClick} selectable={false}>
+          {slice && (
+            <Menu.Item key={MENU_KEYS.EDIT_PROPERTIES}>
+              {t('Edit properties')}
+            </Menu.Item>
+          )}
+          <Menu.Item>
+            <ModalTrigger
+              triggerNode={
+                <span data-test="view-query-menu-item">{t('View query')}</span>
+              }
+              modalTitle={t('View query')}
+              beforeOpen={() => beforeOpen('query')}
+              modalBody={renderQueryModalBody()}
+              responsive
+            />
           </Menu.Item>
-        )}
-        <Menu.Item>
-          <ModalTrigger
-            triggerNode={
-              <span data-test="view-query-menu-item">{t('View query')}</span>
-            }
-            modalTitle={t('View query')}
-            beforeOpen={() => beforeOpen('query')}
-            modalBody={renderQueryModalBody()}
-            responsive
-          />
-        </Menu.Item>
-        {sqlSupported && (
-          <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
-            {t('Run in SQL Lab')}
+          {sqlSupported && (
+            <Menu.Item key={MENU_KEYS.RUN_IN_SQL_LAB}>
+              {t('Run in SQL Lab')}
+            </Menu.Item>
+          )}
+          <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
+            {t('Download as image')}
           </Menu.Item>
-        )}
-        <Menu.Item key={MENU_KEYS.DOWNLOAD_AS_IMAGE}>
-          {t('Download as image')}
-        </Menu.Item>
-      </Menu>
-    </DropdownButton>
+        </Menu>
+      }
+    >
+      <div
+        role="button"
+        id="query"
+        tabIndex={0}
+        className="btn btn-default btn-sm"
+      >

Review comment:
       Maybe we can replace this with the shiny new Button component?




----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577890884



##########
File path: superset-frontend/src/dashboard/components/menu/WithPopoverMenu.jsx
##########
@@ -38,7 +38,9 @@ const defaultProps = {
   menuItems: [],
   isFocused: false,
   shouldFocus: (event, container) =>
-    container && container.contains(event.target),
+    (container && container.contains(event.target)) ||

Review comment:
       Could use the null coalescing operator here, 
   ```suggestion
       (container?.contains(event.target)) ||
   ```




----------------------------------------------------------------
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


[GitHub] [superset] rusackas commented on a change in pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #13002:
URL: https://github.com/apache/superset/pull/13002#discussion_r577888262



##########
File path: superset-frontend/src/dashboard/components/menu/PopoverDropdown.jsx
##########
@@ -78,6 +78,7 @@ const MenuItem = styled(Menu.Item)`
 class PopoverDropdown extends React.PureComponent {

Review comment:
       Maybe we should add a Storybook entry? Could be a separate PR, or we can do a whole audit of missing storybook entries after we close the books on some of the current audit/transition efforts.




----------------------------------------------------------------
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


[GitHub] [superset] codecov-io edited a comment on pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #13002:
URL: https://github.com/apache/superset/pull/13002#issuecomment-775288673






----------------------------------------------------------------
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


[GitHub] [superset] junlincc commented on pull request #13002: refactor: Bootstrap to AntD - DropdownButton

Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #13002:
URL: https://github.com/apache/superset/pull/13002#issuecomment-775705255


   not related, @michael-s-molina Michael, could you double check if we covered the casing change in the main menu dropdown? thanks! 
   <img width="221" alt="Screen Shot 2021-02-08 at 10 29 28 PM" src="https://user-images.githubusercontent.com/67837651/107324561-23cbe580-6a5d-11eb-88a0-702d7c01b32f.png">
   


----------------------------------------------------------------
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