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/11/29 16:26:18 UTC

[GitHub] [superset] lyndsiWilliams opened a new pull request, #22262: fix: Create dataset polish/bug fix

lyndsiWilliams opened a new pull request, #22262:
URL: https://github.com/apache/superset/pull/22262

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   This PR implements the following polishes:
   - Pagination on the table should be 25 by default (previously set to 10)
   -  Align refresh buttons in the left panel
   -  Align width of selected tables in left panel with search input
       - The width was only wrong when there was no scrollbar present. I implemented a function to detect if a scrollbar is present and apply padding only if there is no scrollbar
   -  Remove save button and `...` dropdown menu from the top right of the header
       - An `editing` prop was added in the header component so that it can be reused when building the edit dataset component
   - Brought the "Manage your databases here" blank state from SQL lab to the left panel
   - Fixed a bug where the table wouldn't take the full height of the dataset panel when there is no "This table already has a dataset" banner at the top
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   #### BEFORE:
   <img width="1189" alt="createDSbefore" src="https://user-images.githubusercontent.com/55605634/204584095-c895da39-a00b-41a1-8fa7-b4f45ec1e4e2.png">
   
   #### AFTER:
   <img width="1189" alt="1234createDSpolish" src="https://user-images.githubusercontent.com/55605634/204584222-75cfe0a0-dda9-4c0b-a19f-18294649794d.png">
   
   ![5datasetDBselectorBlankState](https://user-images.githubusercontent.com/55605634/204584253-f5b1fa5d-3ac9-4782-a4f3-0de38992d6ac.png)
   
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   - Go to `http://localhost:9000/dataset/add/?testing`
   - Select a database and a schema
       - Select a schema with over 25 pages to see default for pagination
   - Observe all the changes described above
   
   ### 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] lyndsiWilliams commented on a diff in pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22262:
URL: https://github.com/apache/superset/pull/22262#discussion_r1040357155


##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx:
##########
@@ -82,19 +85,25 @@ export default function Header({
 
   return (
     <HeaderComponentStyles>
-      <PageHeaderWithActions
-        editableTitleProps={editableTitleProps}
-        showTitlePanelItems={false}
-        showFaveStar={false}
-        faveStarProps={{ itemId: 1, saveFaveStar: () => {} }}
-        titlePanelAdditionalItems={<></>}
-        rightPanelAdditionalItems={renderDisabledSaveButton()}
-        additionalActionsMenu={renderOverlay()}
-        menuDropdownProps={{
-          disabled: true,
-        }}
-        tooltipProps={tooltipProps}
-      />
+      {editing ? (
+        <PageHeaderWithActions
+          editableTitleProps={editableTitleProps}
+          showTitlePanelItems={false}
+          showFaveStar={false}
+          faveStarProps={{ itemId: 1, saveFaveStar: () => {} }}
+          titlePanelAdditionalItems={<></>}
+          rightPanelAdditionalItems={renderDisabledSaveButton()}
+          additionalActionsMenu={renderOverlay()}
+          menuDropdownProps={{
+            disabled: true,
+          }}
+          tooltipProps={tooltipProps}
+        />
+      ) : (
+        <StyledCreateDatasetTitle>
+          {title ?? DEFAULT_TITLE}

Review Comment:
   Good call! Fixed in [`this commit`](https://github.com/apache/superset/pull/22262/commits/924f67e1a8068722e9d3872cd1a97af260076b6e).



-- 
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] AAfghahi commented on a diff in pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #22262:
URL: https://github.com/apache/superset/pull/22262#discussion_r1051016100


##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -95,6 +95,28 @@ const collapseStyles = (theme: SupersetTheme) => css`
   }
 `;
 
+const NO_DATABASES_MATCH_TITLE = t('No databases match your search');
+const NO_DATABASES_AVAILABLE_TITLE = t('There are no databases available');
+const MANAGE_YOUR_DATABASES_TEXT = t('Manage your databases');
+const HERE_TEXT = t('here');
+
+export const emptyStateComponent = (emptyResultsWithSearch: boolean) => (

Review Comment:
   If this is going to be exported, I think that we should move it out of the SQlEditorLeftBar? Where are the other empty state components stored? THat's where it should be. 



-- 
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 #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22262:
URL: https://github.com/apache/superset/pull/22262#issuecomment-1359625479

   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


[GitHub] [superset] github-actions[bot] commented on pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22262:
URL: https://github.com/apache/superset/pull/22262#issuecomment-1331210642

   @lyndsiWilliams Ephemeral environment spinning up at http://52.24.161.135: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] lyndsiWilliams commented on pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #22262:
URL: https://github.com/apache/superset/pull/22262#issuecomment-1331207481

   /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] Antonio-RiveroMartnez commented on a diff in pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22262:
URL: https://github.com/apache/superset/pull/22262#discussion_r1040079927


##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx:
##########
@@ -82,19 +85,25 @@ export default function Header({
 
   return (
     <HeaderComponentStyles>
-      <PageHeaderWithActions
-        editableTitleProps={editableTitleProps}
-        showTitlePanelItems={false}
-        showFaveStar={false}
-        faveStarProps={{ itemId: 1, saveFaveStar: () => {} }}
-        titlePanelAdditionalItems={<></>}
-        rightPanelAdditionalItems={renderDisabledSaveButton()}
-        additionalActionsMenu={renderOverlay()}
-        menuDropdownProps={{
-          disabled: true,
-        }}
-        tooltipProps={tooltipProps}
-      />
+      {editing ? (
+        <PageHeaderWithActions
+          editableTitleProps={editableTitleProps}
+          showTitlePanelItems={false}
+          showFaveStar={false}
+          faveStarProps={{ itemId: 1, saveFaveStar: () => {} }}
+          titlePanelAdditionalItems={<></>}
+          rightPanelAdditionalItems={renderDisabledSaveButton()}
+          additionalActionsMenu={renderOverlay()}
+          menuDropdownProps={{
+            disabled: true,
+          }}
+          tooltipProps={tooltipProps}
+        />
+      ) : (
+        <StyledCreateDatasetTitle>
+          {title ?? DEFAULT_TITLE}

Review Comment:
   Do we want to render empty string if passed as `title`? If not, we should change this to be `||`



##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx:
##########
@@ -248,6 +258,9 @@ const DatasetPanel = ({
   const theme = useTheme();
   const hasColumns = columnList?.length > 0 ?? false;
   const datasetNames = datasets?.map(dataset => dataset.table_name);
+  const tablesWithDatasets = datasets?.find(

Review Comment:
   should we rename this to singular? `tablesWithDatasets` -> `tableWithDatasets` since `find` only returns one element?



-- 
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 #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22262:
URL: https://github.com/apache/superset/pull/22262#issuecomment-1331035956

   # [Codecov](https://codecov.io/gh/apache/superset/pull/22262?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 [#22262](https://codecov.io/gh/apache/superset/pull/22262?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a42b75f) into [lyndsi/tables-with-preexisting-datasets](https://codecov.io/gh/apache/superset/commit/0bed85aa39b40be715a33d3d8db62ac8ca2eb853?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0bed85a) will **decrease** coverage by `14.48%`.
   > The diff coverage is `92.50%`.
   
   ```diff
   @@                             Coverage Diff                              @@
   ##           lyndsi/tables-with-preexisting-datasets   #22262       +/-   ##
   ============================================================================
   - Coverage                                    67.02%   52.54%   -14.49%     
   ============================================================================
     Files                                         1835     1835               
     Lines                                        70531    70531               
     Branches                                      7755     7755               
   ============================================================================
   - Hits                                         47274    37059    -10215     
   - Misses                                       21250    31465    +10215     
     Partials                                      2007     2007               
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `?` | |
   | python | `50.85% <ø> (-30.53%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `50.85% <ø> (ø)` | |
   
   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/22262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...iews/CRUD/data/dataset/AddDataset/Header/index.tsx](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXQvSGVhZGVyL2luZGV4LnRzeA==) | `81.81% <0.00%> (ø)` | |
   | [...a/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXQvRGF0YXNldFBhbmVsL0RhdGFzZXRQYW5lbC50c3g=) | `86.95% <90.90%> (ø)` | |
   | [...d/src/SqlLab/components/SqlEditorLeftBar/index.tsx](https://codecov.io/gh/apache/superset/pull/22262/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) | `61.06% <93.33%> (ø)` | |
   | [...s/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L0FkZERhdGFzZXQvTGVmdFBhbmVsL2luZGV4LnRzeA==) | `87.32% <100.00%> (ø)` | |
   | [...set-frontend/src/views/CRUD/data/dataset/styles.ts](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhc2V0L3N0eWxlcy50cw==) | `100.00% <100.00%> (ø)` | |
   | [superset/examples/css\_templates.py](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQvZXhhbXBsZXMvY3NzX3RlbXBsYXRlcy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/migrations/shared/migrate\_viz/\_\_init\_\_.py](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQvbWlncmF0aW9ucy9zaGFyZWQvbWlncmF0ZV92aXovX19pbml0X18ucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...perset/migrations/shared/migrate\_viz/processors.py](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQvbWlncmF0aW9ucy9zaGFyZWQvbWlncmF0ZV92aXovcHJvY2Vzc29ycy5weQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [superset/reports/commands/alert.py](https://codecov.io/gh/apache/superset/pull/22262/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-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9hbGVydC5weQ==) | `0.00% <0.00%> (-96.60%)` | :arrow_down: |
   | ... and [316 more](https://codecov.io/gh/apache/superset/pull/22262/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) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] hughhhh commented on a diff in pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22262:
URL: https://github.com/apache/superset/pull/22262#discussion_r1051015163


##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx:
##########
@@ -271,16 +284,33 @@ const DatasetPanel = ({
       component = (
         <>
           <StyledTitle>{COLUMN_TITLE}</StyledTitle>
-          <TableContainer>
-            <StyledTable
-              loading={loading}
-              size={TableSize.SMALL}
-              columns={tableColumnDefinition}
-              data={columnList}
-              pageSizeOptions={pageSizeOptions}
-              defaultPageSize={10}
-            />
-          </TableContainer>
+          {tableWithDataset ? (
+            <TableContainerWithBanner>
+              <TableScrollContainer>
+                <Table
+                  loading={loading}
+                  size={TableSize.SMALL}
+                  columns={tableColumnDefinition}
+                  data={columnList}
+                  pageSizeOptions={pageSizeOptions}
+                  defaultPageSize={25}
+                />
+              </TableScrollContainer>
+            </TableContainerWithBanner>
+          ) : (
+            <TableContainerWithoutBanner>
+              <TableScrollContainer>
+                <Table
+                  loading={loading}
+                  size={TableSize.SMALL}
+                  columns={tableColumnDefinition}
+                  data={columnList}
+                  pageSizeOptions={pageSizeOptions}
+                  defaultPageSize={25}

Review Comment:
   can we make this a constant at the top?



-- 
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] lyndsiWilliams commented on a diff in pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22262:
URL: https://github.com/apache/superset/pull/22262#discussion_r1040357042


##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx:
##########
@@ -248,6 +258,9 @@ const DatasetPanel = ({
   const theme = useTheme();
   const hasColumns = columnList?.length > 0 ?? false;
   const datasetNames = datasets?.map(dataset => dataset.table_name);
+  const tablesWithDatasets = datasets?.find(

Review Comment:
   Oh yeah that does make more sense, adjusted in [`this commit`](https://github.com/apache/superset/pull/22262/commits/924f67e1a8068722e9d3872cd1a97af260076b6e).



-- 
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] AAfghahi merged pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
AAfghahi merged PR #22262:
URL: https://github.com/apache/superset/pull/22262


-- 
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] lyndsiWilliams commented on a diff in pull request #22262: fix: Create dataset polish/bug fix

Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22262:
URL: https://github.com/apache/superset/pull/22262#discussion_r1052348653


##########
superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx:
##########
@@ -271,16 +284,33 @@ const DatasetPanel = ({
       component = (
         <>
           <StyledTitle>{COLUMN_TITLE}</StyledTitle>
-          <TableContainer>
-            <StyledTable
-              loading={loading}
-              size={TableSize.SMALL}
-              columns={tableColumnDefinition}
-              data={columnList}
-              pageSizeOptions={pageSizeOptions}
-              defaultPageSize={10}
-            />
-          </TableContainer>
+          {tableWithDataset ? (
+            <TableContainerWithBanner>
+              <TableScrollContainer>
+                <Table
+                  loading={loading}
+                  size={TableSize.SMALL}
+                  columns={tableColumnDefinition}
+                  data={columnList}
+                  pageSizeOptions={pageSizeOptions}
+                  defaultPageSize={25}
+                />
+              </TableScrollContainer>
+            </TableContainerWithBanner>
+          ) : (
+            <TableContainerWithoutBanner>
+              <TableScrollContainer>
+                <Table
+                  loading={loading}
+                  size={TableSize.SMALL}
+                  columns={tableColumnDefinition}
+                  data={columnList}
+                  pageSizeOptions={pageSizeOptions}
+                  defaultPageSize={25}

Review Comment:
   Can do! Done in [`this commit`](https://github.com/apache/superset/pull/22262/commits/985f6a919628df1135aa322ff08510b18af6ca3d).



##########
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx:
##########
@@ -95,6 +95,28 @@ const collapseStyles = (theme: SupersetTheme) => css`
   }
 `;
 
+const NO_DATABASES_MATCH_TITLE = t('No databases match your search');
+const NO_DATABASES_AVAILABLE_TITLE = t('There are no databases available');
+const MANAGE_YOUR_DATABASES_TEXT = t('Manage your databases');
+const HERE_TEXT = t('here');
+
+export const emptyStateComponent = (emptyResultsWithSearch: boolean) => (

Review Comment:
   Good thinking! Done in [`this commit`](https://github.com/apache/superset/pull/22262/commits/985f6a919628df1135aa322ff08510b18af6ca3d).



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