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 2022/04/07 19:02:34 UTC
[GitHub] [superset] pkdotson opened a new pull request, #19598: feat: add empty states to sqlab editor and select
pkdotson opened a new pull request, #19598:
URL: https://github.com/apache/superset/pull/19598
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
This pr creates an empty state for the dropdown for the table selector and query editor when there is no database selected.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
after
https://user-images.githubusercontent.com/17326228/162277445-cefc2bdf-6ee5-4ce4-912d-dafb85dda4fc.mov
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Go to sql editor. If there is no db selected to query there should be an empty state for the query editor. If there is no db connected to db select dropdown should show empty state with link to foward user to db listview.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] github-actions[bot] commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1100298960
@yousoph Ephemeral environment spinning up at http://34.219.6.87:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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[bot] commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1095911968
# [Codecov](https://codecov.io/gh/apache/superset/pull/19598?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#19598](https://codecov.io/gh/apache/superset/pull/19598?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bae0137) into [master](https://codecov.io/gh/apache/superset/commit/d693f4e9700e932a29cb51583b26e10793aeab17?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d693f4e) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #19598 +/- ##
=======================================
Coverage 66.30% 66.30%
=======================================
Files 1681 1681
Lines 64408 64461 +53
Branches 6593 6613 +20
=======================================
+ Hits 42704 42741 +37
- Misses 20020 20024 +4
- Partials 1684 1696 +12
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `51.16% <100.00%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/19598?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/components/Select/Select.tsx](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1NlbGVjdC50c3g=) | `87.28% <ø> (ø)` | |
| [...et-frontend/src/components/TableSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGVTZWxlY3Rvci9pbmRleC50c3g=) | `68.29% <ø> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.61% <100.00%> (+0.22%)` | :arrow_up: |
| [...frontend/src/SqlLab/components/SqlEditor/index.jsx](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci9pbmRleC5qc3g=) | `49.43% <100.00%> (-1.41%)` | :arrow_down: |
| [...d/src/SqlLab/components/SqlEditorLeftBar/index.tsx](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvckxlZnRCYXIvaW5kZXgudHN4) | `60.00% <100.00%> (+3.24%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `33.51% <100.00%> (+0.36%)` | :arrow_up: |
| [...frontend/src/components/DatabaseSelector/index.tsx](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRGF0YWJhc2VTZWxlY3Rvci9pbmRleC50c3g=) | `90.00% <100.00%> (+1.29%)` | :arrow_up: |
| [superset-frontend/src/views/CRUD/utils.tsx](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvdXRpbHMudHN4) | `64.51% <0.00%> (-1.62%)` | :arrow_down: |
| [superset-frontend/src/views/components/SubMenu.tsx](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL2NvbXBvbmVudHMvU3ViTWVudS50c3g=) | `85.96% <0.00%> (-1.54%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/apache/superset/pull/19598/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19598?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19598?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d693f4e...bae0137](https://codecov.io/gh/apache/superset/pull/19598?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] yousoph commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
yousoph commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1100230585
/testenv up
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r851459947
##########
superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx:
##########
@@ -224,6 +219,29 @@ test('Should database select display options', async () => {
expect(await screen.findByText('test-mysql')).toBeInTheDocument();
});
+test('should show empty state if there are no options', async () => {
+ SupersetClientGet.mockImplementation(
+ async () => ({ json: { result: [] } } as any),
+ );
+ const props = createProps();
+ render(
+ <DatabaseSelector
+ {...props}
+ db={undefined}
+ emptyState={<EmptyStateSmall title="empty" image="" />}
+ />,
+ { useRedux: true },
+ );
+ const select = screen.getByRole('combobox', {
+ name: 'Select database or type database name',
+ });
+ userEvent.click(select);
+ // const emptystate = getByAltText('empty');
Review Comment:
```suggestion
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] github-actions[bot] commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1100233147
@yousoph Ephemeral environment spinning up at http://34.222.112.28:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r850617071
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.jsx:
##########
@@ -227,7 +231,11 @@ class SqlEditor extends React.PureComponent {
// We need to measure the height of the sql editor post render to figure the height of
// the south pane so it gets rendered properly
// eslint-disable-next-line react/no-did-mount-set-state
+ const db = this.props.database;
this.setState({ height: this.getSqlEditorHeight() });
+ if (!db || isEmpty(db)) {
+ this.setState({ showEmptyState: true });
Review Comment:
```suggestion
this.setEmptyState(true);
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1092116986
@pkdotson Tip: If you leave a blank line before the video URL in the PR description, Github will show the video instead of the URL.
```
after
<blank_line> <--- insert a blank line
https://user-images.githubusercontent.com/17326228/162277445-cefc2bdf-6ee5-4ce4-912d-dafb85dda4fc.mov
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r850617715
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.jsx:
##########
@@ -753,10 +765,19 @@ class SqlEditor extends React.PureComponent {
queryEditor={this.props.queryEditor}
tables={this.props.tables}
actions={this.props.actions}
+ setEmptyState={this.setEmptyState}
/>
</div>
</CSSTransition>
- {this.queryPane()}
+ {this.state.showEmptyState ? (
+ <EmptyStateBig
+ image="vector.svg"
+ title="Select a database to write a query"
+ description="Choose one of the available databases from the panel on the left."
Review Comment:
```suggestion
title={t("Select a database to write a query")}
description={t("Choose one of the available databases from the panel on the left.")}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] github-actions[bot] commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1098598352
@yousoph Ephemeral environment spinning up at http://52.40.6.161:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r849796995
##########
superset-frontend/src/components/DatabaseSelector/index.tsx:
##########
@@ -88,6 +95,8 @@ type SchemaValue = { label: string; value: string };
interface DatabaseSelectorProps {
db?: DatabaseObject;
+ dbSelectClosedState?: Dispatch<SetStateAction<boolean>>;
Review Comment:
Maybe instead of `dbSelectClosedState` and `setDbSearch`, you could have a property called `onEmptyResults` that accepts a nullable `searchedText`. Then you can determine the text for `emptyState`. Like this:
```
interface DatabaseSelectorProps {
...
emptyState?: ReactNode;
onEmptyResults?: (searchedText?: string) => void;
}
```
This also inverts the flow of function calls. The component does not know anything about the listeners or state actions.
I still think the best solution would be just to have the `emptyState` property with a generic message like "No databases found" that can be used when the user is opening the select or searching.
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.jsx:
##########
@@ -756,7 +757,15 @@ class SqlEditor extends React.PureComponent {
/>
</div>
</CSSTransition>
- {this.queryPane()}
+ {this.props.sqlLab.dbConnect ? (
+ this.queryPane()
+ ) : (
+ <EmptyStateBig
+ image="vector.svg"
+ title="Select a database to write a query"
+ description="Choose one of the available database on the left"
Review Comment:
```suggestion
title={t('Select a database to write a query')}
description={t('Choose one of the available database on the left')}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] pkdotson commented on a diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
pkdotson commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r847892051
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -92,6 +93,7 @@ export default function SqlEditorLeftBar({
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
+ const [noDbFoundTitle, setTitleForNoDBFound] = useState(false);
Review Comment:
Yep
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] github-actions[bot] commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1096985523
@jinghua-qa Ephemeral environment spinning up at http://34.222.138.157:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] github-actions[bot] commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1098583955
@yousoph Ephemeral environment spinning up at http://54.191.109.78:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] yousoph commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
yousoph commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1098596562
/testenv up
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] sadpandajoe commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
sadpandajoe commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1101825378
🏷️ preset:2022.15
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] yousoph commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
yousoph commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1098582401
/testenv up
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] yousoph commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
yousoph commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1100296858
/testenv up
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] hughhhh commented on a diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r847818353
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -92,6 +93,7 @@ export default function SqlEditorLeftBar({
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
+ const [noDbFoundTitle, setTitleForNoDBFound] = useState(false);
Review Comment:
can we change this to `isDBConnected` instead?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1095391355
> But this component is only being used in Sqllab and used in changing the database. It makes sense for the action to happen here as this where that action is happening.
This component is also used by Explore and the dataset listing.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] betodealmeida commented on a diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r847835044
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -142,6 +144,22 @@ export default function SqlEditorLeftBar({
const shouldShowReset = window.location.search === '?reset=1';
const tableMetaDataHeight = height - 130; // 130 is the height of the selects above
+ const emptyStateComponent = (
+ <EmptyStateSmall
+ image="empty.svg"
+ title={
+ noDbFoundTitle
+ ? 'No databases match your search'
+ : 'There are no databases available'
+ }
+ description={
+ <p>
+ Manage your database <a href="/databaseview/list">here</a>
Review Comment:
These should be translatable with `t()`.
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -92,6 +93,7 @@ export default function SqlEditorLeftBar({
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
+ const [noDbFoundTitle, setTitleForNoDBFound] = useState(false);
Review Comment:
Yeah, the name `noDbFoundTitle` makes me thing this is a string, not a boolean.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] pkdotson merged pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
pkdotson merged PR #19598:
URL: https://github.com/apache/superset/pull/19598
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r851201621
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -88,15 +99,24 @@ export default function SqlEditorLeftBar({
queryEditor,
tables = [],
height = 500,
+ setEmptyState,
}: SqlEditorLeftBarProps) {
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
+ const [isDbSearch, setDbSearch] = useState(false);
Review Comment:
```suggestion
const [emptyResultsWithSearch, setEmptyResultsWithSearch] = useState(false);
```
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -88,15 +99,24 @@ export default function SqlEditorLeftBar({
queryEditor,
tables = [],
height = 500,
+ setEmptyState,
}: SqlEditorLeftBarProps) {
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
+ const [isDbSearch, setDbSearch] = useState(false);
+
useEffect(() => {
queryEditorRef.current = queryEditor;
}, [queryEditor]);
+ const onDbSearch = (searchText?: string) => {
+ if (searchText) setDbSearch(true);
+ else setDbSearch(false);
+ };
Review Comment:
```suggestion
const onEmptyResults = (searchText?: string) => {
setEmptyResultsWithSearch(!!searchText);
};
```
##########
superset-frontend/src/components/DatabaseSelector/index.tsx:
##########
@@ -88,11 +88,13 @@ type SchemaValue = { label: string; value: string };
interface DatabaseSelectorProps {
Review Comment:
```suggestion
export interface DatabaseSelectorProps {
```
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -164,6 +184,22 @@ export default function SqlEditorLeftBar({
const shouldShowReset = window.location.search === '?reset=1';
const tableMetaDataHeight = height - 130; // 130 is the height of the selects above
+ const emptyStateComponent = (
+ <EmptyStateSmall
+ image="empty.svg"
+ title={
+ isDbSearch
Review Comment:
```suggestion
emptyResultsWithSearch
```
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -185,6 +221,8 @@ export default function SqlEditorLeftBar({
return (
<div className="SqlEditorLeftBar">
<TableSelectorMultiple
+ onEmptyResults={onDbSearch}
Review Comment:
```suggestion
onEmptyResults={onEmptyResults}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r850625249
##########
superset-frontend/src/components/TableSelector/index.tsx:
##########
@@ -92,6 +95,8 @@ interface TableSelectorProps {
onTablesLoad?: (options: Array<any>) => void;
readOnly?: boolean;
schema?: string;
+ setEmptyState?: Dispatch<SetStateAction<boolean>>;
Review Comment:
```suggestion
```
##########
superset-frontend/src/components/TableSelector/index.tsx:
##########
@@ -22,6 +22,8 @@ import React, {
ReactNode,
useMemo,
useEffect,
+ Dispatch,
+ SetStateAction,
Review Comment:
```suggestion
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r850621599
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -185,6 +227,10 @@ export default function SqlEditorLeftBar({
return (
<div className="SqlEditorLeftBar">
<TableSelectorMultiple
+ onEmptyResults={onDbSearch}
+ emptyState={emptyStateComponent}
+ setEmptyState={setEmptyState}
+ dbSelectClosedState={setSelectIsClosed}
Review Comment:
```suggestion
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r847590146
##########
superset-frontend/src/SqlLab/components/SqlEditor/index.jsx:
##########
@@ -756,7 +758,16 @@ class SqlEditor extends React.PureComponent {
/>
</div>
</CSSTransition>
- {this.queryPane()}
+ {this.props.sqlLab.dbConnect
+ ?
+ this.queryPane()
+ :
+ <EmptyStateBig
+ image="vector.svg"
+ title="Select a database to write a query"
+ description="Choose one of the avaialable database on the left"
Review Comment:
```suggestion
description="Choose one of the available database on the left"
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] pkdotson commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
pkdotson commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1095386179
> @pkdotson The `DatabaseSelector` is a generic component meant to be used by multiple modules. That's why it's inside `src/components`. You shouldn't add SQL Lab specific logic inside this component like you're doing when interacting with Redux. All this logic should be encapsulated in a SQL Lab component that uses the `DatabaseSelector` and pass a `notFoundContent` property when applicable.
But this component is only being used in Sqllab and used in changing the database. It makes sense for the action to happen here as this where that action is happening.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] jinghua-qa commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1096943132
/testenv up
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r850621260
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -88,15 +99,29 @@ export default function SqlEditorLeftBar({
queryEditor,
tables = [],
height = 500,
+ setEmptyState,
}: SqlEditorLeftBarProps) {
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
+ const [isDbSearch, setDbSearch] = useState(false);
+ const [isSelectIsClosed, setSelectIsClosed] = useState(true);
Review Comment:
```suggestion
```
##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -88,15 +99,29 @@ export default function SqlEditorLeftBar({
queryEditor,
tables = [],
height = 500,
+ setEmptyState,
}: SqlEditorLeftBarProps) {
// Ref needed to avoid infinite rerenders on handlers
// that require and modify the queryEditor
const queryEditorRef = useRef<QueryEditor>(queryEditor);
+ const [isDbSearch, setDbSearch] = useState(false);
+ const [isSelectIsClosed, setSelectIsClosed] = useState(true);
+
useEffect(() => {
queryEditorRef.current = queryEditor;
}, [queryEditor]);
+ useEffect(() => {
+ if (isSelectIsClosed) setDbSearch(false);
+ }, [isSelectIsClosed]);
Review Comment:
```suggestion
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r850624206
##########
superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx:
##########
@@ -224,6 +224,18 @@ test('Should database select display options', async () => {
expect(await screen.findByText('test-mysql')).toBeInTheDocument();
});
+test('should show empty state if there are no options', async () => {
+ const props = createProps();
+ // @ts-ignore
+ props.db = null;
+ render(<DatabaseSelector {...props} />, { useRedux: true });
+ const select = screen.getByRole('combobox', {
+ name: 'Select database or type database name',
+ });
+ userEvent.click(select);
+ expect(screen.queryByText('test-mysql')).not.toBeInTheDocument();
Review Comment:
Right now we are only checking for the absence of the test-mysql text but there is no check about the empty text content.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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 diff in pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #19598:
URL: https://github.com/apache/superset/pull/19598#discussion_r850624817
##########
superset-frontend/src/components/DatabaseSelector/index.tsx:
##########
@@ -88,14 +95,17 @@ type SchemaValue = { label: string; value: string };
interface DatabaseSelectorProps {
db?: DatabaseObject;
+ emptyState?: ReactNode;
formMode?: boolean;
getDbList?: (arg0: any) => {};
handleError: (msg: string) => void;
isDatabaseSelectEnabled?: boolean;
onDbChange?: (db: DatabaseObject) => void;
+ onEmptyResults?: (searchText?: string) => null;
onSchemaChange?: (schema?: string) => void;
onSchemasLoad?: (schemas: Array<object>) => void;
readOnly?: boolean;
+ setDbSearch?: Dispatch<SetStateAction<boolean>>;
Review Comment:
```suggestion
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] github-actions[bot] commented on pull request #19598: feat: add empty states to sqlab editor and select
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #19598:
URL: https://github.com/apache/superset/pull/19598#issuecomment-1100437452
Ephemeral environment shutdown and build artifacts deleted.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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