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 2020/09/28 15:42:28 UTC
[GitHub] [incubator-superset] kgabryje opened a new pull request #11090: [WIP] refactor: Replace react-bootstrap tabs with Antd tabs
kgabryje opened a new pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090
### SUMMARY
Replace react-bootstrap tabs with Antd tabs in profile, `SouthPane` and `TabbedSqlEditor`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
Storybooks for new components were added
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] 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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497667634
##########
File path: superset-frontend/src/common/components/Dropdown.tsx
##########
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Dropdown as AntdDropdown } from 'src/common/components';
+import { css } from '@emotion/core';
+import { styled } from '@superset-ui/core';
+
+const dotStyle = css`
+ width: 3px;
+ height: 3px;
+ border-radius: 1.5px;
+ background-color: #bababa;
+`;
+
+const MenuDots = styled.div`
+ ${dotStyle};
+ font-weight: 400;
Review comment:
```suggestion
font-weight: ${({ theme }) => theme.typography.weights.normal};
```
----------------------------------------------------------------
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] [incubator-superset] kgabryje commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r498291529
##########
File path: superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
##########
@@ -296,123 +302,98 @@ class TabbedSqlEditors extends React.PureComponent {
}
const state = latestQuery ? latestQuery.state : '';
- const title = (
- <>
- {qe.title} <TabStatusIcon tabState={state} />{' '}
- <Icon
- role="button"
- tabIndex={0}
- cursor="pointer"
- name="cancel-x"
+ const menu = (
+ <Menu>
+ <Menu.Item
+ className="close-btn"
+ key="1"
onClick={() => this.removeQueryEditor(qe)}
- />
- </>
+ data-test="close-tab-menu-option"
+ >
+ <div className="icon-container">
+ <i className="fa fa-close" />
+ </div>
+ {t('Close tab')}
+ </Menu.Item>
+ <Menu.Item key="2" onClick={() => this.renameTab(qe)}>
+ <div className="icon-container">
+ <i className="fa fa-i-cursor" />
+ </div>
+ {t('Rename tab')}
+ </Menu.Item>
+ <Menu.Item key="3" onClick={this.toggleLeftBar}>
+ <div className="icon-container">
+ <i className="fa fa-cogs" />
+ </div>
+ {this.state.hideLeftBar ? t('Expand tool bar') : t('Hide tool bar')}
+ </Menu.Item>
+ <Menu.Item
+ key="4"
+ onClick={() => this.removeAllOtherQueryEditors(qe)}
+ >
+ <div className="icon-container">
+ <i className="fa fa-times-circle-o" />
+ </div>
+ {t('Close all other tabs')}
+ </Menu.Item>
+ <Menu.Item key="5" onClick={() => this.duplicateQueryEditor(qe)}>
+ <div className="icon-container">
+ <i className="fa fa-files-o" />
+ </div>
+ {t('Duplicate tab')}
+ </Menu.Item>
+ </Menu>
);
+
const tabTitle = (
<>
- {isSelected && (
- <DropdownButton
- data-test="dropdown-toggle-button"
- bsSize="small"
- id={`ddbtn-tab-${i}`}
- title={' '}
- noCaret
- >
- <MenuItem
- className="close-btn"
- eventKey="1"
- onClick={() => this.removeQueryEditor(qe)}
- data-test="close-tab-menu-option"
- >
- <div className="icon-container">
- <i className="fa fa-close" />
- </div>
- {t('Close tab')}
- </MenuItem>
- <MenuItem eventKey="2" onClick={() => this.renameTab(qe)}>
- <div className="icon-container">
- <i className="fa fa-i-cursor" />
- </div>
- {t('Rename tab')}
- </MenuItem>
- <MenuItem eventKey="3" onClick={this.toggleLeftBar}>
- <div className="icon-container">
- <i className="fa fa-cogs" />
- </div>
- {this.state.hideLeftBar
- ? t('Expand tool bar')
- : t('Hide tool bar')}
- </MenuItem>
- <MenuItem
- eventKey="4"
- onClick={() => this.removeAllOtherQueryEditors(qe)}
- >
- <div className="icon-container">
- <i className="fa fa-times-circle-o" />
- </div>
- {t('Close all other tabs')}
- </MenuItem>
- <MenuItem
- eventKey="5"
- onClick={() => this.duplicateQueryEditor(qe)}
- >
- <div className="icon-container">
- <i className="fa fa-files-o" />
- </div>
- {t('Duplicate tab')}
- </MenuItem>
- </DropdownButton>
- )}
- <span className="ddbtn-tab">{title}</span>
+ <div data-test="dropdown-toggle-button">
+ <Dropdown overlay={menu} trigger={['click']} />
+ </div>
+ <span style={{ marginRight: '8px' }}>{qe.title}</span>{' '}
Review comment:
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] [incubator-superset] codecov-commenter edited a comment on pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-701246491
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=h1) Report
> Merging [#11090](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5d08a426d30e7e3b32e09541937f466889c2319b?el=desc) will **decrease** coverage by `1.06%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11090 +/- ##
==========================================
- Coverage 61.17% 60.10% -1.07%
==========================================
Files 384 385 +1
Lines 24287 24321 +34
==========================================
- Hits 14858 14619 -239
- Misses 9429 9702 +273
```
| Flag | Coverage Δ | |
|---|---|---|
| #python | `60.10% <ø> (-1.07%)` | :arrow_down: |
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/incubator-superset/pull/11090?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `69.76% <0.00%> (-12.20%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/databases/api.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `81.38% <0.00%> (-7.98%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| [superset/views/database/validators.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvdmFsaWRhdG9ycy5weQ==) | `78.94% <0.00%> (-5.27%)` | :arrow_down: |
| ... and [24 more](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11090?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/incubator-superset/pull/11090?src=pr&el=footer). Last update [5d08a42...6a7fd94](https://codecov.io/gh/apache/incubator-superset/pull/11090?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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497678417
##########
File path: superset-frontend/src/common/components/Tabs.tsx
##########
@@ -59,4 +85,48 @@ const Tabs = Object.assign(StyledTabs, {
TabPane: StyledTabPane,
});
+Tabs.defaultProps = {
+ fullWidth: true,
+};
+
+const StyledEditableTabs = styled(StyledTabs)`
Review comment:
We should add this to the Storybook entry for tabs (as another Story, if that makes sense)
----------------------------------------------------------------
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] [incubator-superset] mistercrunch commented on pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-702424051
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497669129
##########
File path: superset-frontend/src/common/components/Dropdown.tsx
##########
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Dropdown as AntdDropdown } from 'src/common/components';
+import { css } from '@emotion/core';
+import { styled } from '@superset-ui/core';
+
+const dotStyle = css`
+ width: 3px;
+ height: 3px;
+ border-radius: 1.5px;
+ background-color: #bababa;
+`;
+
+const MenuDots = styled.div`
+ ${dotStyle};
+ font-weight: 400;
+ display: inline-flex;
+
+ &:hover {
+ background-color: #20a7c9;
+
+ &::before,
+ &::after {
+ background-color: #20a7c9;
+ }
+ }
+
+ &::before,
+ &::after {
+ position: absolute;
+ content: ' ';
+ ${dotStyle};
+ }
+
+ &::before {
+ transform: translateY(-5px);
+ }
+
+ &::after {
+ transform: translateY(5px);
Review comment:
```suggestion
transform: translateY(${({ theme }) => theme.gridUnit}px);
```
Again not sure if 4px is close enough
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497669447
##########
File path: superset-frontend/src/common/components/Dropdown.tsx
##########
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Dropdown as AntdDropdown } from 'src/common/components';
+import { css } from '@emotion/core';
+import { styled } from '@superset-ui/core';
+
+const dotStyle = css`
+ width: 3px;
+ height: 3px;
+ border-radius: 1.5px;
+ background-color: #bababa;
+`;
+
+const MenuDots = styled.div`
+ ${dotStyle};
+ font-weight: 400;
+ display: inline-flex;
+
+ &:hover {
+ background-color: #20a7c9;
+
+ &::before,
+ &::after {
+ background-color: #20a7c9;
+ }
+ }
+
+ &::before,
+ &::after {
+ position: absolute;
+ content: ' ';
+ ${dotStyle};
+ }
+
+ &::before {
+ transform: translateY(-5px);
+ }
+
+ &::after {
+ transform: translateY(5px);
+ }
+`;
+
+const MenuDotsWrapper = styled.div`
+ display: flex;
+ align-items: center;
+ padding: 8px 8px 8px 4px;
Review comment:
```suggestion
padding: ${({ theme }) => theme.gridUnit * 2}px;
padding-left: ${({ theme }) => theme.gridUnit}px;
```
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497668121
##########
File path: superset-frontend/src/common/components/Dropdown.tsx
##########
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Dropdown as AntdDropdown } from 'src/common/components';
+import { css } from '@emotion/core';
+import { styled } from '@superset-ui/core';
+
+const dotStyle = css`
+ width: 3px;
+ height: 3px;
+ border-radius: 1.5px;
+ background-color: #bababa;
+`;
+
+const MenuDots = styled.div`
+ ${dotStyle};
+ font-weight: 400;
+ display: inline-flex;
+
+ &:hover {
+ background-color: #20a7c9;
Review comment:
```suggestion
background-color: ${({ theme }) => theme.colors.primary.base};
```
----------------------------------------------------------------
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] [incubator-superset] rusackas merged pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas merged pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter commented on pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs [WIP]
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-701246491
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=h1) Report
> Merging [#11090](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5d08a426d30e7e3b32e09541937f466889c2319b?el=desc) will **decrease** coverage by `0.26%`.
> The diff coverage is `69.81%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11090 +/- ##
==========================================
- Coverage 61.17% 60.90% -0.27%
==========================================
Files 384 819 +435
Lines 24287 38797 +14510
Branches 0 3654 +3654
==========================================
+ Hits 14858 23631 +8773
- Misses 9429 14986 +5557
- Partials 0 180 +180
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.25% <69.81%> (?)` | |
| #python | `60.10% <ø> (-1.07%)` | :arrow_down: |
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/incubator-superset/pull/11090?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/SqlLab/components/SouthPane.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NvdXRoUGFuZS5qc3g=) | `81.81% <0.00%> (ø)` | |
| [...-frontend/src/common/components/common.stories.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL2NvbW1vbi5zdG9yaWVzLnRzeA==) | `0.00% <0.00%> (ø)` | |
| [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `60.60% <ø> (ø)` | |
| [...rontend/src/SqlLab/components/TabbedSqlEditors.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMuanN4) | `72.66% <61.11%> (ø)` | |
| [...perset-frontend/src/common/components/Dropdown.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL0Ryb3Bkb3duLnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/common/components/Tabs.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL1RhYnMudHN4) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/profile/components/App.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Byb2ZpbGUvY29tcG9uZW50cy9BcHAudHN4) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `31.91% <0.00%> (-59.58%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| ... and [465 more](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11090?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/incubator-superset/pull/11090?src=pr&el=footer). Last update [5d08a42...fb9a2aa](https://codecov.io/gh/apache/incubator-superset/pull/11090?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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497678072
##########
File path: superset-frontend/src/common/components/Tabs.tsx
##########
@@ -16,27 +16,53 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
import { styled } from '@superset-ui/core';
import { Tabs as AntdTabs } from 'src/common/components';
+import { css } from '@emotion/core';
+import Icon from '../../components/Icon';
-const StyledTabs = styled(AntdTabs)`
- margin-top: -18px;
+interface TabsProps {
+ inModal?: boolean;
+ fullWidth?: boolean;
+}
- .ant-tabs-nav-list {
- width: 100%;
- }
+const notForwardedProps = ['fullWidth', 'inModal'];
+
+const StyledTabs = styled(AntdTabs, {
+ shouldForwardProp: prop => !notForwardedProps.includes(prop),
+})<TabsProps>`
+ ${props =>
+ props.inModal &&
Review comment:
These styles should probably be added to the Modal, since it's more the concern of the modal, and the Tabs shouldn't be concerned where they live.
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497668917
##########
File path: superset-frontend/src/common/components/Dropdown.tsx
##########
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Dropdown as AntdDropdown } from 'src/common/components';
+import { css } from '@emotion/core';
+import { styled } from '@superset-ui/core';
+
+const dotStyle = css`
+ width: 3px;
+ height: 3px;
+ border-radius: 1.5px;
+ background-color: #bababa;
+`;
+
+const MenuDots = styled.div`
+ ${dotStyle};
+ font-weight: 400;
+ display: inline-flex;
+
+ &:hover {
+ background-color: #20a7c9;
+
+ &::before,
+ &::after {
+ background-color: #20a7c9;
+ }
+ }
+
+ &::before,
+ &::after {
+ position: absolute;
+ content: ' ';
+ ${dotStyle};
+ }
+
+ &::before {
+ transform: translateY(-5px);
Review comment:
```suggestion
transform: translateY(-${({ theme }) => theme.gridUnit}px);
```
Not sure if 4px is close enough for this use case
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497679715
##########
File path: superset-frontend/src/common/components/Tabs.tsx
##########
@@ -59,4 +85,48 @@ const Tabs = Object.assign(StyledTabs, {
TabPane: StyledTabPane,
});
+Tabs.defaultProps = {
+ fullWidth: true,
+};
+
+const StyledEditableTabs = styled(StyledTabs)`
Review comment:
Nevermind, you did that!!!! 🎉
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497661882
##########
File path: superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
##########
@@ -249,17 +252,23 @@ class TabbedSqlEditors extends React.PureComponent {
}
handleSelect(key) {
- if (key === 'add_tab') {
+ const qeid = this.props.tabHistory[this.props.tabHistory.length - 1];
Review comment:
It looks like the means to add a new query tab is nice and clean to me, but this is one in particular that could use a screenshot/gif.
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497653524
##########
File path: superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
##########
@@ -18,18 +18,19 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
-import { MenuItem, DropdownButton, Tab, Tabs } from 'react-bootstrap';
+import { EditableTabs } from 'src/common/components/Tabs';
+import { Dropdown } from 'src/common/components/Dropdown';
Review comment:
Nice! A new component has entered the game! Might be nice to see a screenshot of this on the 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] [incubator-superset] kgabryje commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r498253905
##########
File path: superset-frontend/src/common/components/Tabs.tsx
##########
@@ -16,27 +16,53 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
import { styled } from '@superset-ui/core';
import { Tabs as AntdTabs } from 'src/common/components';
+import { css } from '@emotion/core';
+import Icon from '../../components/Icon';
-const StyledTabs = styled(AntdTabs)`
- margin-top: -18px;
+interface TabsProps {
+ inModal?: boolean;
+ fullWidth?: boolean;
+}
- .ant-tabs-nav-list {
- width: 100%;
- }
+const notForwardedProps = ['fullWidth', 'inModal'];
+
+const StyledTabs = styled(AntdTabs, {
+ shouldForwardProp: prop => !notForwardedProps.includes(prop),
+})<TabsProps>`
+ ${props =>
+ props.inModal &&
Review comment:
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] [incubator-superset] mistercrunch commented on pull request #11090: [WIP] refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-700388915
Nice. I can't wait to see the new tabs all over the app!
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497657468
##########
File path: superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
##########
@@ -18,18 +18,19 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
-import { MenuItem, DropdownButton, Tab, Tabs } from 'react-bootstrap';
+import { EditableTabs } from 'src/common/components/Tabs';
+import { Dropdown } from 'src/common/components/Dropdown';
+import { Menu } from 'src/common/components';
Review comment:
This one should also probably be carried across... I'll ticket this as well.
----------------------------------------------------------------
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] [incubator-superset] kgabryje commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r498256681
##########
File path: superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
##########
@@ -249,17 +252,23 @@ class TabbedSqlEditors extends React.PureComponent {
}
handleSelect(key) {
- if (key === 'add_tab') {
+ const qeid = this.props.tabHistory[this.props.tabHistory.length - 1];
Review comment:
In the previous version we used a "fake" tab for adding new tabs. AntD tabs ship this functionality out of the box so we don't need this hack with "add_tab" tab. I'll put some before/after screenshots in PR summary
----------------------------------------------------------------
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] [incubator-superset] kgabryje commented on pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-702145383
@rusackas Thank you for your comments! I applied all of your suggested changes
----------------------------------------------------------------
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] [incubator-superset] codecov-commenter edited a comment on pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs [WIP]
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-701246491
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=h1) Report
> Merging [#11090](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5d08a426d30e7e3b32e09541937f466889c2319b?el=desc) will **decrease** coverage by `0.06%`.
> The diff coverage is `69.81%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/11090?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #11090 +/- ##
==========================================
- Coverage 61.17% 61.11% -0.07%
==========================================
Files 384 819 +435
Lines 24287 38808 +14521
Branches 0 3654 +3654
==========================================
+ Hits 14858 23718 +8860
- Misses 9429 14910 +5481
- Partials 0 180 +180
```
| Flag | Coverage Δ | |
|---|---|---|
| #javascript | `62.25% <69.81%> (?)` | |
| #python | `60.43% <ø> (-0.74%)` | :arrow_down: |
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/incubator-superset/pull/11090?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...erset-frontend/src/SqlLab/components/SouthPane.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NvdXRoUGFuZS5qc3g=) | `81.81% <0.00%> (ø)` | |
| [...-frontend/src/common/components/common.stories.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL2NvbW1vbi5zdG9yaWVzLnRzeA==) | `0.00% <0.00%> (ø)` | |
| [...end/src/views/CRUD/data/database/DatabaseModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsLnRzeA==) | `60.60% <ø> (ø)` | |
| [...rontend/src/SqlLab/components/TabbedSqlEditors.jsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMuanN4) | `72.66% <61.11%> (ø)` | |
| [...perset-frontend/src/common/components/Dropdown.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL0Ryb3Bkb3duLnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/common/components/Tabs.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL1RhYnMudHN4) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/profile/components/App.tsx](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Byb2ZpbGUvY29tcG9uZW50cy9BcHAudHN4) | `100.00% <100.00%> (ø)` | |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `53.90% <0.00%> (-30.08%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `79.59% <0.00%> (-12.25%)` | :arrow_down: |
| ... and [451 more](https://codecov.io/gh/apache/incubator-superset/pull/11090/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/11090?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/incubator-superset/pull/11090?src=pr&el=footer). Last update [5d08a42...fb9a2aa](https://codecov.io/gh/apache/incubator-superset/pull/11090?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] [incubator-superset] kgabryje edited a comment on pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
kgabryje edited a comment on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-702145383
@rusackas Thank you for your comments! I applied all of your suggested changes and added screenshots/gifs to PRs summary
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497668220
##########
File path: superset-frontend/src/common/components/Dropdown.tsx
##########
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Dropdown as AntdDropdown } from 'src/common/components';
+import { css } from '@emotion/core';
+import { styled } from '@superset-ui/core';
+
+const dotStyle = css`
+ width: 3px;
+ height: 3px;
+ border-radius: 1.5px;
+ background-color: #bababa;
+`;
+
+const MenuDots = styled.div`
+ ${dotStyle};
+ font-weight: 400;
+ display: inline-flex;
+
+ &:hover {
+ background-color: #20a7c9;
+
+ &::before,
+ &::after {
+ background-color: #20a7c9;
Review comment:
```suggestion
background-color: ${({ theme }) => theme.colors.primary.base};
```
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497678841
##########
File path: superset-frontend/src/common/components/Tabs.tsx
##########
@@ -59,4 +85,48 @@ const Tabs = Object.assign(StyledTabs, {
TabPane: StyledTabPane,
});
+Tabs.defaultProps = {
+ fullWidth: true,
+};
+
+const StyledEditableTabs = styled(StyledTabs)`
+ .ant-tabs-content-holder {
+ background: white;
+ }
+
+ & > .ant-tabs-nav {
+ margin-bottom: 0;
+ }
+
+ .ant-tabs-tab-remove {
+ padding-top: 0;
+ padding-bottom: 0;
+ height: 24px;
Review comment:
```suggestion
height: ${({ theme }) => theme.gridUnit * 6}px;
```
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497654171
##########
File path: superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
##########
@@ -18,18 +18,19 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
-import { MenuItem, DropdownButton, Tab, Tabs } from 'react-bootstrap';
+import { EditableTabs } from 'src/common/components/Tabs';
+import { Dropdown } from 'src/common/components/Dropdown';
Review comment:
And actually, we should probably make a ticket to restyle this to match our design system, and use it everywhere, replacing the React Bootstrap implementation.
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on a change in pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on a change in pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#discussion_r497663098
##########
File path: superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
##########
@@ -296,123 +302,98 @@ class TabbedSqlEditors extends React.PureComponent {
}
const state = latestQuery ? latestQuery.state : '';
- const title = (
- <>
- {qe.title} <TabStatusIcon tabState={state} />{' '}
- <Icon
- role="button"
- tabIndex={0}
- cursor="pointer"
- name="cancel-x"
+ const menu = (
+ <Menu>
+ <Menu.Item
+ className="close-btn"
+ key="1"
onClick={() => this.removeQueryEditor(qe)}
- />
- </>
+ data-test="close-tab-menu-option"
+ >
+ <div className="icon-container">
+ <i className="fa fa-close" />
+ </div>
+ {t('Close tab')}
+ </Menu.Item>
+ <Menu.Item key="2" onClick={() => this.renameTab(qe)}>
+ <div className="icon-container">
+ <i className="fa fa-i-cursor" />
+ </div>
+ {t('Rename tab')}
+ </Menu.Item>
+ <Menu.Item key="3" onClick={this.toggleLeftBar}>
+ <div className="icon-container">
+ <i className="fa fa-cogs" />
+ </div>
+ {this.state.hideLeftBar ? t('Expand tool bar') : t('Hide tool bar')}
+ </Menu.Item>
+ <Menu.Item
+ key="4"
+ onClick={() => this.removeAllOtherQueryEditors(qe)}
+ >
+ <div className="icon-container">
+ <i className="fa fa-times-circle-o" />
+ </div>
+ {t('Close all other tabs')}
+ </Menu.Item>
+ <Menu.Item key="5" onClick={() => this.duplicateQueryEditor(qe)}>
+ <div className="icon-container">
+ <i className="fa fa-files-o" />
+ </div>
+ {t('Duplicate tab')}
+ </Menu.Item>
+ </Menu>
);
+
const tabTitle = (
<>
- {isSelected && (
- <DropdownButton
- data-test="dropdown-toggle-button"
- bsSize="small"
- id={`ddbtn-tab-${i}`}
- title={' '}
- noCaret
- >
- <MenuItem
- className="close-btn"
- eventKey="1"
- onClick={() => this.removeQueryEditor(qe)}
- data-test="close-tab-menu-option"
- >
- <div className="icon-container">
- <i className="fa fa-close" />
- </div>
- {t('Close tab')}
- </MenuItem>
- <MenuItem eventKey="2" onClick={() => this.renameTab(qe)}>
- <div className="icon-container">
- <i className="fa fa-i-cursor" />
- </div>
- {t('Rename tab')}
- </MenuItem>
- <MenuItem eventKey="3" onClick={this.toggleLeftBar}>
- <div className="icon-container">
- <i className="fa fa-cogs" />
- </div>
- {this.state.hideLeftBar
- ? t('Expand tool bar')
- : t('Hide tool bar')}
- </MenuItem>
- <MenuItem
- eventKey="4"
- onClick={() => this.removeAllOtherQueryEditors(qe)}
- >
- <div className="icon-container">
- <i className="fa fa-times-circle-o" />
- </div>
- {t('Close all other tabs')}
- </MenuItem>
- <MenuItem
- eventKey="5"
- onClick={() => this.duplicateQueryEditor(qe)}
- >
- <div className="icon-container">
- <i className="fa fa-files-o" />
- </div>
- {t('Duplicate tab')}
- </MenuItem>
- </DropdownButton>
- )}
- <span className="ddbtn-tab">{title}</span>
+ <div data-test="dropdown-toggle-button">
+ <Dropdown overlay={menu} trigger={['click']} />
+ </div>
+ <span style={{ marginRight: '8px' }}>{qe.title}</span>{' '}
Review comment:
No need to do it now, but pointing out that this is one of a million components with inline `style` properties that I'd like to replace with Emotion... then `8px` can be `...{...gridUnit*2}px` so we can adjust it with a theme.
----------------------------------------------------------------
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] [incubator-superset] rusackas commented on pull request #11090: refactor: Replace react-bootstrap tabs with Antd tabs
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #11090:
URL: https://github.com/apache/incubator-superset/pull/11090#issuecomment-702934717
This looks great! Thanks for all the hard work on this!
----------------------------------------------------------------
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