Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update search in datasource panel to use matchSorter #12319

Merged
merged 2 commits into from
Jan 7, 2021

Conversation

pkdotson
Copy link
Member

@pkdotson pkdotson commented Jan 7, 2021

SUMMARY

Datasource search was case sensitive and did not re-search on character delete. This pr adds matchSorter for search of metrics and columns characters while do all character checking.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-01-06.at.5.21.50.PM.mov

TEST PLAN

ADDITIONAL INFORMATION

@codecov-io
Copy link

codecov-io commented Jan 7, 2021

Codecov Report

Merging #12319 (3772d4b) into master (176f54d) will decrease coverage by 4.14%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12319      +/-   ##
==========================================
- Coverage   67.14%   63.00%   -4.15%     
==========================================
  Files        1002     1002              
  Lines       49300    49283      -17     
  Branches     5010     5010              
==========================================
- Hits        33104    31050    -2054     
- Misses      16071    18031    +1960     
- Partials      125      202      +77     
Flag Coverage Δ
cypress ?
javascript 61.22% <50.00%> (+0.01%) ⬆️
python 64.16% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...rontend/src/explore/components/DatasourcePanel.tsx 85.36% <50.00%> (+3.54%) ⬆️
superset-frontend/src/SqlLab/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/dashboard/App.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/setup/setupColors.js 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/chart/ChartContainer.jsx 0.00% <0.00%> (-100.00%) ⬇️
superset-frontend/src/explore/reducers/index.js 0.00% <0.00%> (-100.00%) ⬇️
...et-frontend/src/dashboard/containers/Dashboard.jsx 0.00% <0.00%> (-100.00%) ⬇️
...t-frontend/src/dashboard/containers/SliceAdder.jsx 0.00% <0.00%> (-100.00%) ⬇️
...t-frontend/src/explore/reducers/getInitialState.js 0.00% <0.00%> (-100.00%) ⬇️
... and 188 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 176f54d...3772d4b. Read the comment docs.

@junlincc junlincc added the rush! Requires immediate attention label Jan 7, 2021
@mistercrunch mistercrunch merged commit 506edf4 into apache:master Jan 7, 2021
@mistercrunch mistercrunch deleted the fix-search branch January 7, 2021 05:56
setList({ columns: filteredColumns, metrics: filteredMetrics });
setList({
columns: matchSorter(columns, value, { keys: ['column_name'] }),
metrics: matchSorter(metrics, value, { keys: ['metric_name'] }),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want to search by SQL expression and data type as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you mean searching metrics and columns properties, which users can't really see?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. But I think they do have a way to see both. For metric definition, there is an info icon tooltip, for data type there is an icon (maybe not that useful than the metric definition, though).

@junlincc junlincc added the explore:datapanel Related to the Data panel of Explore label Jan 8, 2021
@junlincc junlincc removed the rush! Requires immediate attention label Mar 18, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels explore:datapanel Related to the Data panel of Explore preset-io size/S 🚢 1.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants