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

[GitHub] [incubator-superset] graceguo-supercat opened a new pull request #10210: [WIP] Typeahead dashboard filter_box

graceguo-supercat opened a new pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210


   ### SUMMARY
   Test a prototype for typeahead searchable filter_box for dashboard.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


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

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



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

[GitHub] [incubator-superset] graceguo-supercat commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-653146650


   @mistercrunch @rusackas @zuzana-vej @sylvia-tomiyama I would like to hear your suggestion about:
   
   - the control name `Fixed Filter Options` and description.
   - when this feature should be enabled? Currently it is an option and chart owner can set it for a filter key. @ktmud suggest we should auto enable this when filter has > 1000 options. My concern is because auto-completes trigger a new query, if the datasource is made of many table joins, the response will be very slow. If we automatically enable this for all large filter box, there is no way for slow ones being escaped.
   - Note: auto-completes only happens when user type-in. If user only load page, open filter_box, play with options in the initial load, this will not trigger extra query.
   


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

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



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

[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r449391782



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Fixed Filter Options')}
+          tooltip={t(
+            'By default, filter box will only load top 1000 filter options once. ' +
+              'When disabled, filter box will load options from a remote source as the user types, ' +
+              'with 1000 options per load.',

Review comment:
       !danger here, it's very possible that you'd be running dozens of `SELECT DISTINCT dimension_column FROM VERY_LARGE_TABLE WHERE dimension_column LIKE '%{str}%'`.  Your poor Presto cluster might suffer.
   
   Also the cache hit rate would be very low compared to the normal filter. 




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

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



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

[GitHub] [incubator-superset] graceguo-supercat edited a comment on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-654372805


   we like `Search All Filter Options`. also updated description:
   <img width="439" alt="Screen Shot 2020-07-06 at 10 27 29 AM" src="https://user-images.githubusercontent.com/27990562/86621741-9333a100-bf73-11ea-99db-591e16157933.png">
   
   As description said, uncheck by default. Only send extra query when feature is enabled, `and` column has > 1000 initial options.
   
   


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

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



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

[GitHub] [incubator-superset] mistercrunch commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-653369743


   I want to make sure people see this comment https://github.com/apache/incubator-superset/pull/10210#discussion_r449391782


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

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



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

[GitHub] [incubator-superset] rusackas commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-652790567


   > The name `Fixed Filter Options` isn't very clear. How about `Dynamic Autocomplete` ?
   
   Names aside, I like the idea of checking the box to turn on the query feature, rather than turning on the limitation.
   
   From a UX perspective, having that enabled by default seems like it would cause less pain/confusion, if it's not too "expensive"
   
   And as for the name, that's tough... maybe "Search All Filter Options?"


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

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



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

[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r448730345



##########
File path: superset-frontend/src/visualizations/FilterBox/FilterBox.jsx
##########
@@ -162,6 +178,75 @@ class FilterBox extends React.Component {
     });
   }
 
+  /**
+   * Generate a debounce function that loads options for a specific column
+   */
+  debounceLoadOptions(key) {
+    if (!(key in this.debouncerCache)) {
+      this.debouncerCache[key] = debounce((input, callback) => {
+        this.loadOptions(key, input).then(callback);
+      }, 500);

Review comment:
       use a large delay (500ms) to reduce number of queries sent to backend.




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

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



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

[GitHub] [incubator-superset] graceguo-supercat edited a comment on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-654372805


   we like `Search All Filter Options`. also updated description:
   <img width="439" alt="Screen Shot 2020-07-06 at 10 27 29 AM" src="https://user-images.githubusercontent.com/27990562/86621741-9333a100-bf73-11ea-99db-591e16157933.png">
   
   As description said, uncheck by default. Only send extra query when feature is enabled, and a column has > 1000 initial options.
   
   


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

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



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

[GitHub] [incubator-superset] graceguo-supercat edited a comment on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-652763923


   > The name `Fixed Filter Options` isn't very clear. How about `Dynamic Autocomplete` ?
   
   even with fixed number of options, when user type in, filter_box can do `autocomplete`. But it only auto-completes with the 1000 options already returned to front-end.


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

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



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

[GitHub] [incubator-superset] codecov-commenter commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-654402976


   # [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10210?src=pr&el=h1) Report
   > Merging [#10210](https://codecov.io/gh/apache/incubator-superset/pull/10210?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/96647054351535843dc802401a834f64358e99e4&el=desc) will **increase** coverage by `4.45%`.
   > The diff coverage is `45.09%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-superset/pull/10210/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/incubator-superset/pull/10210?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #10210      +/-   ##
   ==========================================
   + Coverage   65.69%   70.15%   +4.45%     
   ==========================================
     Files         594      594              
     Lines       31497    31668     +171     
     Branches     3223     3237      +14     
   ==========================================
   + Hits        20692    22216    +1524     
   + Misses      10625     9344    -1281     
   + Partials      180      108      -72     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | #cypress | `52.89% <41.17%> (?)` | |
   | #javascript | `59.30% <5.88%> (-0.13%)` | :arrow_down: |
   | #python | `69.80% <ø> (-0.36%)` | :arrow_down: |
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10210?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...end/src/visualizations/FilterBox/transformProps.js](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL0ZpbHRlckJveC90cmFuc2Zvcm1Qcm9wcy5qcw==) | `78.57% <ø> (+78.57%)` | :arrow_up: |
   | [...rontend/src/visualizations/FilterBox/FilterBox.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL0ZpbHRlckJveC9GaWx0ZXJCb3guanN4) | `66.25% <42.55%> (+61.33%)` | :arrow_up: |
   | [...plore/components/controls/FilterBoxItemControl.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJCb3hJdGVtQ29udHJvbC5qc3g=) | `74.07% <66.66%> (-1.40%)` | :arrow_down: |
   | [superset-frontend/src/explore/constants.js](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29uc3RhbnRzLmpz) | `100.00% <100.00%> (ø)` | |
   | [superset/views/database/views.py](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.35% <0.00%> (-28.24%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `78.72% <0.00%> (-12.77%)` | :arrow_down: |
   | [superset/views/database/forms.py](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `78.37% <0.00%> (-12.32%)` | :arrow_down: |
   | [superset/utils/log.py](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvbG9nLnB5) | `92.94% <0.00%> (-3.06%)` | :arrow_down: |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `83.98% <0.00%> (-2.78%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.83% <0.00%> (-0.84%)` | :arrow_down: |
   | ... and [165 more](https://codecov.io/gh/apache/incubator-superset/pull/10210/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10210?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10210?src=pr&el=footer). Last update [9664705...811a977](https://codecov.io/gh/apache/incubator-superset/pull/10210?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


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

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



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

[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r448722762



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Fixed Filter Options')}
+          tooltip={t(
+            'By default, filter box will only load top 1000 filter options once. ' +
+              'When disabled, filter box will load options from a remote source as the user types, ' +
+              'with 1000 options per load.',

Review comment:
       let me check with PM tomorrow.




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

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



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

[GitHub] [incubator-superset] mistercrunch edited a comment on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
mistercrunch edited a comment on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-652747388


   The name `Fixed Filter Options` isn't very clear. How about `Dynamic Autocomplete` ?
   


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

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



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

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r450478645



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Search All Filter Options')}
+          tooltip={t(
+            'By default, filter box loads top 1000 records. Check this box if you want to ' +
+              'enable dynamically searching and loading filter values as user types ' +
+              '(applicable only when the filter returns more than 1000 records).',

Review comment:
       Suggestion:
   
   > By default, each filter loads at most 1000 choices at the initial page load. Check this box if you have more than 1000 filter values and want to enable dynamically searching that loads filter values as users type (may add stress to your database).




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

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



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

[GitHub] [incubator-superset] ktmud commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-653263163


   > The name `Fixed Filter Options` isn't very clear. How about `Dynamic Autocomplete` ?
   
   +1 to "Dynamic Autocomplete".
   
   


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

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



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

[GitHub] [incubator-superset] graceguo-supercat edited a comment on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-653146650


   @mistercrunch @rusackas @zuzana-vej @sylvia-tomiyama I would like to hear your suggestion about:
   
   - the control name `Fixed Filter Options` and description.
   - when this feature should be enabled? Currently it is an option and chart owner can enable fetch extra options for a filter key. @ktmud suggest we should auto enable this when filter has > 1000 options. My concern is because auto-completion will trigger a new query, if the datasource is made of many table joins, the response will be very slow. If we automatically enable this for all large filter box, there is no way for slow ones being escaped.
   - Note: auto-completes only happens when user type-in. If user only load page, open filter_box, play with options in the initial load, this will not trigger extra query.
   


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

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



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

[GitHub] [incubator-superset] graceguo-supercat commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-652763923


   > The name `Fixed Filter Options` isn't very clear. How about `Dynamic Autocomplete` ?
   
   even with fixed number of options, when user type in, filter_box can do `autocomplete`. But it only auto-completes with options already returned to front-end.


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

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



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

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r450478645



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Search All Filter Options')}
+          tooltip={t(
+            'By default, filter box loads top 1000 records. Check this box if you want to ' +
+              'enable dynamically searching and loading filter values as user types ' +
+              '(applicable only when the filter returns more than 1000 records).',

Review comment:
       Suggestion:
   
   > By default, each filter loads at most 1000 choices at the initial page load. Check this box if you have more than 1000 filter values and want to enable dynamically searching that loads filter values as users type.




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

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



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

[GitHub] [incubator-superset] graceguo-supercat edited a comment on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat edited a comment on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-654372805


   we like `Search All Filter Options`. also updated description:
   <img width="422" alt="Screen Shot 2020-07-07 at 9 29 35 AM" src="https://user-images.githubusercontent.com/27990562/86812938-70b58c80-c034-11ea-89a5-b29704b4e330.png">
   
   
   As description said, uncheck by default. Only send extra query when feature is enabled, `and` column has > 1000 initial options.
   
   


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

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



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

[GitHub] [incubator-superset] rusackas edited a comment on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
rusackas edited a comment on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-652790567


   > The name `Fixed Filter Options` isn't very clear. How about `Dynamic Autocomplete` ?
   
   Names aside, I like the idea of checking the box to turn on the query feature, rather than turning on the limitation.
   
   From a UX perspective, having that enabled by default seems like it would cause less pain/confusion, if it's not too "expensive"
   
   And as for the name, that's tough... maybe "Search All Filter Options?" Wordy, but explicit.


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

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



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

[GitHub] [incubator-superset] graceguo-supercat commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-654372805


   we like `Search All Filter Options`. also updated description:
   <img width="439" alt="Screen Shot 2020-07-06 at 10 27 29 AM" src="https://user-images.githubusercontent.com/27990562/86621741-9333a100-bf73-11ea-99db-591e16157933.png">
   
   As description said, uncheck by default, `and` only send extra query when a column has > 1000 initial options.
   
   


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

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



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

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r448720490



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Fixed Filter Options')}
+          tooltip={t(
+            'By default, filter box will only load top 1000 filter options once. ' +
+              'When disabled, filter box will load options from a remote source as the user types, ' +
+              'with 1000 options per load.',

Review comment:
       Should we make the async loading the default or allow Superset admins to change it?




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

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



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

[GitHub] [incubator-superset] mistercrunch commented on pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#issuecomment-652747388


   The name `Fixed Filter Options` isn't very clear. I'm not sure what to call it though. 


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

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



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

[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r450557257



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Search All Filter Options')}
+          tooltip={t(
+            'By default, filter box loads top 1000 records. Check this box if you want to ' +
+              'enable dynamically searching and loading filter values as user types ' +
+              '(applicable only when the filter returns more than 1000 records).',

Review comment:
       description is updated.




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

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



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

[GitHub] [incubator-superset] graceguo-supercat merged pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat merged pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210


   


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

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



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

[GitHub] [incubator-superset] graceguo-supercat commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r448730345



##########
File path: superset-frontend/src/visualizations/FilterBox/FilterBox.jsx
##########
@@ -162,6 +178,75 @@ class FilterBox extends React.Component {
     });
   }
 
+  /**
+   * Generate a debounce function that loads options for a specific column
+   */
+  debounceLoadOptions(key) {
+    if (!(key in this.debouncerCache)) {
+      this.debouncerCache[key] = debounce((input, callback) => {
+        this.loadOptions(key, input).then(callback);
+      }, 500);

Review comment:
       use a large delay to reduce number of queries.




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

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



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

[GitHub] [incubator-superset] ktmud commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r450478645



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Search All Filter Options')}
+          tooltip={t(
+            'By default, filter box loads top 1000 records. Check this box if you want to ' +
+              'enable dynamically searching and loading filter values as user types ' +
+              '(applicable only when the filter returns more than 1000 records).',

Review comment:
       Suggestion:
   
   > By default, each filter loads only at most 1000 choices at the initial page load. Check this box if you have more than 1000 filter values and want to enable dynamically searching that loads filter values as users type.




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

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



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

[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #10210: feat: Typeahead searchable filter_box for dashboard

Posted by GitBox <gi...@apache.org>.
mistercrunch commented on a change in pull request #10210:
URL: https://github.com/apache/incubator-superset/pull/10210#discussion_r449393082



##########
File path: superset-frontend/src/explore/components/controls/FilterBoxItemControl.jsx
##########
@@ -202,6 +220,21 @@ export default class FilterBoxItemControl extends React.Component {
             />
           }
         />
+        <FormRow
+          label={t('Fixed Filter Options')}
+          tooltip={t(
+            'By default, filter box will only load top 1000 filter options once. ' +
+              'When disabled, filter box will load options from a remote source as the user types, ' +
+              'with 1000 options per load.',

Review comment:
       So I'd argue for unchecked by default, and a good explanation of the tradeoff in a toolitp




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