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

[GitHub] [superset] cccs-RyanS opened a new pull request #15153: [WIP] Adding a show all button to the column/metrics list in the explore view (Allow more than 50 columns to be shown)

cccs-RyanS opened a new pull request #15153:
URL: https://github.com/apache/superset/pull/15153


   ### SUMMARY
   This is a simple addition to the explore view. The explore view limits the amount of data / columns displayed at once to 50. We wish to have the option of showing all of the columns / metrics at one time. The look / functionality was discussed in
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   Open a data source with with more than 50 columns or metrics. At the button of there will be a "Show All..." button, clicking this button will cause dull list of columns / metrics to render. If all columns / metrics are rendering a "Show Less..." button will appear, clicking this will cause only 50 elements to render.
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue:
   - [x] 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.

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] cccs-rc commented on a change in pull request #15153: [WIP] Adding a show all button to the column/metrics list in the explore view (Allow more than 50 columns to be shown)

Posted by GitBox <gi...@apache.org>.
cccs-rc commented on a change in pull request #15153:
URL: https://github.com/apache/superset/pull/15153#discussion_r651180481



##########
File path: superset-frontend/src/explore/components/DatasourcePanel/index.tsx
##########
@@ -113,6 +125,11 @@ export default function DataSourcePanel({
     columns,
     metrics,
   });
+  const [showAllMetrics, setShowAllMetrics] = useState(false);
+  const [showAllColumns, setShowAllColumns] = useState(false);
+
+  const DEFAULT_MAX_COLUMNS_LENGTH = 50;
+  const DEFAULT_MAX_METRIC_LENGTH = 50;

Review comment:
       Biggest nitpick ever. 
   ```suggestion
     const DEFAULT_MAX_METRICS_LENGTH = 50;
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [superset] codecov[bot] commented on pull request #15153: feat: Adding a show all button to the column/metrics list in the explore view (Allow more than 50 columns to be shown)

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


   # [Codecov](https://codecov.io/gh/apache/superset/pull/15153?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 [#15153](https://codecov.io/gh/apache/superset/pull/15153?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2f410e0) into [master](https://codecov.io/gh/apache/superset/commit/38660449383b4940e1127520af512fe2e3f9323d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3866044) will **decrease** coverage by `0.24%`.
   > The diff coverage is `57.14%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/15153/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/15153?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #15153      +/-   ##
   ==========================================
   - Coverage   77.41%   77.17%   -0.25%     
   ==========================================
     Files         969      971       +2     
     Lines       49918    50296     +378     
     Branches     6393     6150     -243     
   ==========================================
   + Hits        38645    38816     +171     
   - Misses      11070    11272     +202     
   - Partials      203      208       +5     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `71.79% <57.14%> (-0.43%)` | :arrow_down: |
   
   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/15153?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...d/src/explore/components/DatasourcePanel/index.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwvaW5kZXgudHN4) | `81.33% <57.14%> (-9.74%)` | :arrow_down: |
   | [superset-frontend/src/components/Select/utils.ts](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3V0aWxzLnRz) | `66.66% <0.00%> (-27.46%)` | :arrow_down: |
   | [...nfigModal/FiltersConfigForm/CollapsibleControl.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdGb3JtL0NvbGxhcHNpYmxlQ29udHJvbC50c3g=) | `91.30% <0.00%> (-8.70%)` | :arrow_down: |
   | [superset-frontend/src/CRUD/CollectionTable.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvQ29sbGVjdGlvblRhYmxlLnRzeA==) | `78.91% <0.00%> (-6.93%)` | :arrow_down: |
   | [...ters/FiltersConfigModal/FiltersConfigForm/state.ts](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdGb3JtL3N0YXRlLnRz) | `96.66% <0.00%> (-3.34%)` | :arrow_down: |
   | [...ols/DateFilterControl/components/CalendarFrame.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRlRmlsdGVyQ29udHJvbC9jb21wb25lbnRzL0NhbGVuZGFyRnJhbWUudHN4) | `42.85% <0.00%> (-3.30%)` | :arrow_down: |
   | [superset-frontend/src/common/components/index.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL2luZGV4LnRzeA==) | `98.03% <0.00%> (-1.97%)` | :arrow_down: |
   | [...tend/src/SqlLab/components/SouthPane/SouthPane.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NvdXRoUGFuZS9Tb3V0aFBhbmUudHN4) | `78.26% <0.00%> (-1.29%)` | :arrow_down: |
   | [...d/components/DashboardBuilder/DashboardBuilder.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0Rhc2hib2FyZEJ1aWxkZXIvRGFzaGJvYXJkQnVpbGRlci50c3g=) | `89.89% <0.00%> (-1.13%)` | :arrow_down: |
   | [...onfigModal/FiltersConfigForm/FiltersConfigForm.tsx](https://codecov.io/gh/apache/superset/pull/15153/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdGb3JtL0ZpbHRlcnNDb25maWdGb3JtLnRzeA==) | `69.15% <0.00%> (-0.92%)` | :arrow_down: |
   | ... and [35 more](https://codecov.io/gh/apache/superset/pull/15153/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/15153?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/15153?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 [3866044...2f410e0](https://codecov.io/gh/apache/superset/pull/15153?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] cccs-rc commented on a change in pull request #15153: [WIP] Adding a show all button to the column/metrics list in the explore view (Allow more than 50 columns to be shown)

Posted by GitBox <gi...@apache.org>.
cccs-rc commented on a change in pull request #15153:
URL: https://github.com/apache/superset/pull/15153#discussion_r651182354



##########
File path: superset-frontend/src/explore/components/DatasourcePanel/index.tsx
##########
@@ -219,6 +240,15 @@ export default function DataSourcePanel({
                 )}
               </LabelContainer>
             ))}
+            {lists.metrics.length > DEFAULT_MAX_METRIC_LENGTH ? (
+              <ButtonContainer>
+                <Button onClick={() => setShowAllMetrics(!showAllMetrics)}>
+                  {showAllMetrics ? t('Show Less...') : t('Show all...')}

Review comment:
       One more giant nitpick. 
   ```suggestion
                     {showAllMetrics ? t('Show less...') : t('Show all...')}
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


[GitHub] [superset] zhaoyongjie commented on a change in pull request #15153: feat: Adding a show all button to the column/metrics list in the explore view (Allow more than 50 columns to be shown)

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



##########
File path: superset-frontend/src/explore/components/DatasourcePanel/index.tsx
##########
@@ -113,6 +125,11 @@ export default function DataSourcePanel({
     columns,
     metrics,
   });
+  const [showAllMetrics, setShowAllMetrics] = useState(false);
+  const [showAllColumns, setShowAllColumns] = useState(false);
+
+  const DEFAULT_MAX_COLUMNS_LENGTH = 50;

Review comment:
       Should we put these constants in the superset config, then access these values from store of redux `explore.common.conf.DEFAULT_MAX_COLUMNS_LENGTH` and `explore.common.conf.DEFAULT_MAX_METRICS_LENGTH`




-- 
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] zhaoyongjie commented on a change in pull request #15153: feat: Adding a show all button to the column/metrics list in the explore view (Allow more than 50 columns to be shown)

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



##########
File path: superset-frontend/src/explore/components/DatasourcePanel/index.tsx
##########
@@ -113,6 +125,11 @@ export default function DataSourcePanel({
     columns,
     metrics,
   });
+  const [showAllMetrics, setShowAllMetrics] = useState(false);
+  const [showAllColumns, setShowAllColumns] = useState(false);
+
+  const DEFAULT_MAX_COLUMNS_LENGTH = 50;

Review comment:
       Should we put these constants in the superset config, then access these values from store of redux `explore.common.conf.DEFAULT_MAX_COLUMNS_LENGTH` and `explore.common.conf.DEFAULT_MAX_METRICS_LENGTH`




-- 
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] amitmiran137 merged pull request #15153: feat: Adding a show all button to the column/metrics list in the explore view (Allow more than 50 columns to be shown)

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


   


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