[explore] fix autocomplete on verbose names #5204
Conversation
| @@ -224,8 +224,13 @@ export default class MetricsControl extends React.PureComponent { | |||
| (option.column_name.toLowerCase().indexOf(valueAfterAggregate.toLowerCase()) >= 0); | |||
| } | |||
| return option.optionName && | |||
| (!option.metric_name || !this.isAutoGeneratedMetric(option)) && | |||
There was a problem hiding this comment.
@GabeLoins I'm trying to understand !option.metric_name why filtering this out?
There was a problem hiding this comment.
in the metrics control you can select saved_metrics, columns, or aggregates from the dropdown. That's why I use optionName for comparison rather than metric name. In that check I'm saying if the option is not a saved metric, or if it is a saved metric but not an autogenerated saved metric
|
@mistercrunch as of this PR: we only filter simple metrics with names that are formatted as So simple saved metrics with fancy names should still be displayed. |
Currently when searching for metrics or groupbys, the autocomplete search functionality only matches based on the metric_name, though in some cases the verbose_name is displayed and disregarded for search purposes. Also another issue is that not all pre-defined metrics show up in the drop down which is confusing. Users may have simple metrics for which they setup a nice verbose name and/or description and expect to see those in the dropdown. This PR addresses it for metric and column-related dropdowns.
|
Got it, let's get the ones with a |
Codecov Report
@@ Coverage Diff @@
## master #5204 +/- ##
==========================================
+ Coverage 63.39% 63.45% +0.05%
==========================================
Files 261 261
Lines 19838 19842 +4
Branches 1995 1998 +3
==========================================
+ Hits 12577 12591 +14
+ Misses 7252 7242 -10
Partials 9 9
Continue to review full report at Codecov.
|
| @@ -224,7 +224,7 @@ describe('MetricsControl', () => { | |||
|
|
|||
| expect(!!wrapper.instance().selectFilterOption( | |||
| { type: 'VARCHAR(255)', column_name: 'source', optionName: '_col_source' }, | |||
| 'Sou', | |||
| 'sou', | |||
There was a problem hiding this comment.
FYI react-select always passes lower case strings so I removed the toLowerCase call.
|
@GabeLoins please review when you find a moment |
| @@ -103,6 +103,10 @@ const groupByControl = { | |||
| optionRenderer: c => <ColumnOption column={c} showType />, | |||
| valueRenderer: c => <ColumnOption column={c} />, | |||
| valueKey: 'column_name', | |||
| filterOption: (opt, text) => ( | |||
There was a problem hiding this comment.
why does this only apply to the group by control? shouldn't we do the same for metric control?
There was a problem hiding this comment.
MetricsControl has it's own control that internally defines filterOption in it's own way. I alter MetricsControl.selectFilterOption in this very PR to address this.
This reverts commit d5ebc43.
* [explore] fix autocomplete on verbose names Currently when searching for metrics or groupbys, the autocomplete search functionality only matches based on the metric_name, though in some cases the verbose_name is displayed and disregarded for search purposes. Also another issue is that not all pre-defined metrics show up in the drop down which is confusing. Users may have simple metrics for which they setup a nice verbose name and/or description and expect to see those in the dropdown. This PR addresses it for metric and column-related dropdowns. * Add unit test
…5219) * Revert "[explore] fix autocomplete on verbose names (apache#5204)" This reverts commit d5ebc43. * Revert "[webpack] setup lazy loading for all visualizations (apache#4727)" This reverts commit de0aaf4.
* [explore] fix autocomplete on verbose names Currently when searching for metrics or groupbys, the autocomplete search functionality only matches based on the metric_name, though in some cases the verbose_name is displayed and disregarded for search purposes. Also another issue is that not all pre-defined metrics show up in the drop down which is confusing. Users may have simple metrics for which they setup a nice verbose name and/or description and expect to see those in the dropdown. This PR addresses it for metric and column-related dropdowns. * Add unit test
…5219) * Revert "[explore] fix autocomplete on verbose names (apache#5204)" This reverts commit d5ebc43. * Revert "[webpack] setup lazy loading for all visualizations (apache#4727)" This reverts commit de0aaf4.
mistercrunch
added
🏷️ bot
Currently when searching for metrics or groupbys, the autocomplete
search functionality only matches based on the metric_name, though in
some cases the verbose_name is displayed and disregarded for search
purposes.
Also another issue is that not all pre-defined metrics show up in the
drop down which is confusing. Users may have simple metrics for which
they setup a nice verbose name and/or description and expect to see
those in the dropdown.
This PR addresses it for metric and column-related dropdowns.
@GabeLoins @michellethomas